The only known BD70528 use-cases are such that the PMIC is controlled
from separate MCU which is not running Linux. I am not aware of
any Linux driver users. Furthermore, it seems there is no demand for
this IC. Let's ease the maintenance burden and drop the driver. We can
always add it back if there is sudden need for it.
Signed-off-by: Matti Vaittinen <[email protected]>
---
My heart is a bit less bleeding when I see how much simpler this RTC
driver became..
Please let me know if some of you think the driver is needed.
---
drivers/rtc/Kconfig | 4 +-
drivers/rtc/rtc-bd70528.c | 316 ++------------------------------------
2 files changed, 13 insertions(+), 307 deletions(-)
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index d8c13fded164..398899217626 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -502,10 +502,10 @@ config RTC_DRV_M41T80_WDT
config RTC_DRV_BD70528
tristate "ROHM BD70528, BD71815 and BD71828 PMIC RTC"
- depends on MFD_ROHM_BD71828 || MFD_ROHM_BD70528 && (BD70528_WATCHDOG || !BD70528_WATCHDOG)
+ depends on MFD_ROHM_BD71828
help
If you say Y here you will get support for the RTC
- block on ROHM BD70528, BD71815 and BD71828 Power Management IC.
+ block on ROHM BD71815 and BD71828 Power Management IC.
This driver can also be built as a module. If so, the module
will be called rtc-bd70528.
diff --git a/drivers/rtc/rtc-bd70528.c b/drivers/rtc/rtc-bd70528.c
index 6454afca02a6..59b627fc1ecf 100644
--- a/drivers/rtc/rtc-bd70528.c
+++ b/drivers/rtc/rtc-bd70528.c
@@ -2,10 +2,9 @@
//
// Copyright (C) 2018 ROHM Semiconductors
//
-// RTC driver for ROHM BD70528 PMIC
+// RTC driver for ROHM BD71828 and BD71815 PMIC
#include <linux/bcd.h>
-#include <linux/mfd/rohm-bd70528.h>
#include <linux/mfd/rohm-bd71815.h>
#include <linux/mfd/rohm-bd71828.h>
#include <linux/module.h>
@@ -39,11 +38,6 @@ struct bd70528_rtc_data {
u8 year;
} __packed;
-struct bd70528_rtc_wake {
- struct bd70528_rtc_day time;
- u8 ctrl;
-} __packed;
-
struct bd71828_rtc_alm {
struct bd70528_rtc_data alm0;
struct bd70528_rtc_data alm1;
@@ -51,141 +45,14 @@ struct bd71828_rtc_alm {
u8 alm1_mask;
} __packed;
-struct bd70528_rtc_alm {
- struct bd70528_rtc_data data;
- u8 alm_mask;
- u8 alm_repeat;
-} __packed;
-
struct bd70528_rtc {
struct rohm_regmap_dev *parent;
struct regmap *regmap;
struct device *dev;
u8 reg_time_start;
u8 bd718xx_alm_block_start;
- bool has_rtc_timers;
};
-static int bd70528_set_wake(struct rohm_regmap_dev *bd70528,
- int enable, int *old_state)
-{
- int ret;
- unsigned int ctrl_reg;
-
- ret = regmap_read(bd70528->regmap, BD70528_REG_WAKE_EN, &ctrl_reg);
- if (ret)
- return ret;
-
- if (old_state) {
- if (ctrl_reg & BD70528_MASK_WAKE_EN)
- *old_state |= BD70528_WAKE_STATE_BIT;
- else
- *old_state &= ~BD70528_WAKE_STATE_BIT;
-
- if (!enable == !(*old_state & BD70528_WAKE_STATE_BIT))
- return 0;
- }
-
- if (enable)
- ctrl_reg |= BD70528_MASK_WAKE_EN;
- else
- ctrl_reg &= ~BD70528_MASK_WAKE_EN;
-
- return regmap_write(bd70528->regmap, BD70528_REG_WAKE_EN,
- ctrl_reg);
-}
-
-static int bd70528_set_elapsed_tmr(struct rohm_regmap_dev *bd70528,
- int enable, int *old_state)
-{
- int ret;
- unsigned int ctrl_reg;
-
- /*
- * TBD
- * What is the purpose of elapsed timer ?
- * Is the timeout registers counting down, or is the disable - re-enable
- * going to restart the elapsed-time counting? If counting is restarted
- * the timeout should be decreased by the amount of time that has
- * elapsed since starting the timer. Maybe we should store the monotonic
- * clock value when timer is started so that if RTC is set while timer
- * is armed we could do the compensation. This is a hack if RTC/system
- * clk are drifting. OTOH, RTC controlled via I2C is in any case
- * inaccurate...
- */
- ret = regmap_read(bd70528->regmap, BD70528_REG_ELAPSED_TIMER_EN,
- &ctrl_reg);
- if (ret)
- return ret;
-
- if (old_state) {
- if (ctrl_reg & BD70528_MASK_ELAPSED_TIMER_EN)
- *old_state |= BD70528_ELAPSED_STATE_BIT;
- else
- *old_state &= ~BD70528_ELAPSED_STATE_BIT;
-
- if ((!enable) == (!(*old_state & BD70528_ELAPSED_STATE_BIT)))
- return 0;
- }
-
- if (enable)
- ctrl_reg |= BD70528_MASK_ELAPSED_TIMER_EN;
- else
- ctrl_reg &= ~BD70528_MASK_ELAPSED_TIMER_EN;
-
- return regmap_write(bd70528->regmap, BD70528_REG_ELAPSED_TIMER_EN,
- ctrl_reg);
-}
-
-static int bd70528_set_rtc_based_timers(struct bd70528_rtc *r, int new_state,
- int *old_state)
-{
- int ret;
-
- ret = bd70528_wdt_set(r->parent, new_state & BD70528_WDT_STATE_BIT,
- old_state);
- if (ret) {
- dev_err(r->dev,
- "Failed to disable WDG for RTC setting (%d)\n", ret);
- return ret;
- }
- ret = bd70528_set_elapsed_tmr(r->parent,
- new_state & BD70528_ELAPSED_STATE_BIT,
- old_state);
- if (ret) {
- dev_err(r->dev,
- "Failed to disable 'elapsed timer' for RTC setting\n");
- return ret;
- }
- ret = bd70528_set_wake(r->parent, new_state & BD70528_WAKE_STATE_BIT,
- old_state);
- if (ret) {
- dev_err(r->dev,
- "Failed to disable 'wake timer' for RTC setting\n");
- return ret;
- }
-
- return ret;
-}
-
-static int bd70528_re_enable_rtc_based_timers(struct bd70528_rtc *r,
- int old_state)
-{
- if (!r->has_rtc_timers)
- return 0;
-
- return bd70528_set_rtc_based_timers(r, old_state, NULL);
-}
-
-static int bd70528_disable_rtc_based_timers(struct bd70528_rtc *r,
- int *old_state)
-{
- if (!r->has_rtc_timers)
- return 0;
-
- return bd70528_set_rtc_based_timers(r, 0, old_state);
-}
-
static inline void tmday2rtc(struct rtc_time *t, struct bd70528_rtc_day *d)
{
d->sec &= ~BD70528_MASK_RTC_SEC;
@@ -267,52 +134,6 @@ static int bd71828_set_alarm(struct device *dev, struct rtc_wkalrm *a)
}
-static int bd70528_set_alarm(struct device *dev, struct rtc_wkalrm *a)
-{
- struct bd70528_rtc_wake wake;
- struct bd70528_rtc_alm alm;
- int ret;
- struct bd70528_rtc *r = dev_get_drvdata(dev);
-
- ret = regmap_bulk_read(r->regmap, BD70528_REG_RTC_WAKE_START, &wake,
- sizeof(wake));
- if (ret) {
- dev_err(dev, "Failed to read wake regs\n");
- return ret;
- }
-
- ret = regmap_bulk_read(r->regmap, BD70528_REG_RTC_ALM_START, &alm,
- sizeof(alm));
- if (ret) {
- dev_err(dev, "Failed to read alarm regs\n");
- return ret;
- }
-
- tm2rtc(&a->time, &alm.data);
- tmday2rtc(&a->time, &wake.time);
-
- if (a->enabled) {
- alm.alm_mask &= ~BD70528_MASK_ALM_EN;
- wake.ctrl |= BD70528_MASK_WAKE_EN;
- } else {
- alm.alm_mask |= BD70528_MASK_ALM_EN;
- wake.ctrl &= ~BD70528_MASK_WAKE_EN;
- }
-
- ret = regmap_bulk_write(r->regmap, BD70528_REG_RTC_WAKE_START, &wake,
- sizeof(wake));
- if (ret) {
- dev_err(dev, "Failed to set wake time\n");
- return ret;
- }
- ret = regmap_bulk_write(r->regmap, BD70528_REG_RTC_ALM_START, &alm,
- sizeof(alm));
- if (ret)
- dev_err(dev, "Failed to set alarm time\n");
-
- return ret;
-}
-
static int bd71828_read_alarm(struct device *dev, struct rtc_wkalrm *a)
{
int ret;
@@ -336,78 +157,28 @@ static int bd71828_read_alarm(struct device *dev, struct rtc_wkalrm *a)
return 0;
}
-static int bd70528_read_alarm(struct device *dev, struct rtc_wkalrm *a)
+static int bd71828_set_time(struct device *dev, struct rtc_time *t)
{
- struct bd70528_rtc_alm alm;
int ret;
- struct bd70528_rtc *r = dev_get_drvdata(dev);
-
- ret = regmap_bulk_read(r->regmap, BD70528_REG_RTC_ALM_START, &alm,
- sizeof(alm));
- if (ret) {
- dev_err(dev, "Failed to read alarm regs\n");
- return ret;
- }
-
- rtc2tm(&alm.data, &a->time);
- a->time.tm_mday = -1;
- a->time.tm_mon = -1;
- a->time.tm_year = -1;
- a->enabled = !(alm.alm_mask & BD70528_MASK_ALM_EN);
- a->pending = 0;
-
- return 0;
-}
-
-static int bd70528_set_time_locked(struct device *dev, struct rtc_time *t)
-{
- int ret, tmpret, old_states;
struct bd70528_rtc_data rtc_data;
struct bd70528_rtc *r = dev_get_drvdata(dev);
- ret = bd70528_disable_rtc_based_timers(r, &old_states);
- if (ret)
- return ret;
-
- tmpret = regmap_bulk_read(r->regmap, r->reg_time_start, &rtc_data,
- sizeof(rtc_data));
- if (tmpret) {
+ ret = regmap_bulk_read(r->regmap, r->reg_time_start, &rtc_data,
+ sizeof(rtc_data));
+ if (ret) {
dev_err(dev, "Failed to read RTC time registers\n");
- goto renable_out;
+ return ret;
}
tm2rtc(t, &rtc_data);
- tmpret = regmap_bulk_write(r->regmap, r->reg_time_start, &rtc_data,
- sizeof(rtc_data));
- if (tmpret) {
+ ret = regmap_bulk_write(r->regmap, r->reg_time_start, &rtc_data,
+ sizeof(rtc_data));
+ if (ret)
dev_err(dev, "Failed to set RTC time\n");
- goto renable_out;
- }
-
-renable_out:
- ret = bd70528_re_enable_rtc_based_timers(r, old_states);
- if (tmpret)
- ret = tmpret;
return ret;
}
-static int bd71828_set_time(struct device *dev, struct rtc_time *t)
-{
- return bd70528_set_time_locked(dev, t);
-}
-
-static int bd70528_set_time(struct device *dev, struct rtc_time *t)
-{
- int ret;
- struct bd70528_rtc *r = dev_get_drvdata(dev);
-
- bd70528_wdt_lock(r->parent);
- ret = bd70528_set_time_locked(dev, t);
- bd70528_wdt_unlock(r->parent);
- return ret;
-}
-
static int bd70528_get_time(struct device *dev, struct rtc_time *t)
{
struct bd70528_rtc *r = dev_get_drvdata(dev);
@@ -427,31 +198,6 @@ static int bd70528_get_time(struct device *dev, struct rtc_time *t)
return 0;
}
-static int bd70528_alm_enable(struct device *dev, unsigned int enabled)
-{
- int ret;
- unsigned int enableval = BD70528_MASK_ALM_EN;
- struct bd70528_rtc *r = dev_get_drvdata(dev);
-
- if (enabled)
- enableval = 0;
-
- bd70528_wdt_lock(r->parent);
- ret = bd70528_set_wake(r->parent, enabled, NULL);
- if (ret) {
- dev_err(dev, "Failed to change wake state\n");
- goto out_unlock;
- }
- ret = regmap_update_bits(r->regmap, BD70528_REG_RTC_ALM_MASK,
- BD70528_MASK_ALM_EN, enableval);
- if (ret)
- dev_err(dev, "Failed to change alarm state\n");
-
-out_unlock:
- bd70528_wdt_unlock(r->parent);
- return ret;
-}
-
static int bd71828_alm_enable(struct device *dev, unsigned int enabled)
{
int ret;
@@ -470,14 +216,6 @@ static int bd71828_alm_enable(struct device *dev, unsigned int enabled)
return ret;
}
-static const struct rtc_class_ops bd70528_rtc_ops = {
- .read_time = bd70528_get_time,
- .set_time = bd70528_set_time,
- .read_alarm = bd70528_read_alarm,
- .set_alarm = bd70528_set_alarm,
- .alarm_irq_enable = bd70528_alm_enable,
-};
-
static const struct rtc_class_ops bd71828_rtc_ops = {
.read_time = bd70528_get_time,
.set_time = bd71828_set_time,
@@ -503,7 +241,6 @@ static int bd70528_probe(struct platform_device *pdev)
struct rtc_device *rtc;
int irq;
unsigned int hr;
- bool enable_main_irq = false;
u8 hour_reg;
enum rohm_chip_type chip = platform_get_device_id(pdev)->driver_data;
@@ -518,21 +255,9 @@ static int bd70528_probe(struct platform_device *pdev)
}
bd_rtc->dev = &pdev->dev;
+ rtc_ops = &bd71828_rtc_ops;
switch (chip) {
- case ROHM_CHIP_TYPE_BD70528:
- bd_rtc->parent = dev_get_drvdata(pdev->dev.parent);
- if (!bd_rtc->parent) {
- dev_err(&pdev->dev, "No MFD data\n");
- return -EINVAL;
- }
- irq_name = "bd70528-rtc-alm";
- bd_rtc->has_rtc_timers = true;
- bd_rtc->reg_time_start = BD70528_REG_RTC_START;
- hour_reg = BD70528_REG_RTC_HOUR;
- enable_main_irq = true;
- rtc_ops = &bd70528_rtc_ops;
- break;
case ROHM_CHIP_TYPE_BD71815:
irq_name = "bd71815-rtc-alm-0";
bd_rtc->reg_time_start = BD71815_REG_RTC_START;
@@ -549,14 +274,12 @@ static int bd70528_probe(struct platform_device *pdev)
*/
bd_rtc->bd718xx_alm_block_start = BD71815_REG_RTC_ALM_START;
hour_reg = BD71815_REG_HOUR;
- rtc_ops = &bd71828_rtc_ops;
break;
case ROHM_CHIP_TYPE_BD71828:
irq_name = "bd71828-rtc-alm-0";
bd_rtc->reg_time_start = BD71828_REG_RTC_START;
bd_rtc->bd718xx_alm_block_start = BD71828_REG_RTC_ALM_START;
hour_reg = BD71828_REG_RTC_HOUR;
- rtc_ops = &bd71828_rtc_ops;
break;
default:
dev_err(&pdev->dev, "Unknown chip\n");
@@ -611,27 +334,10 @@ static int bd70528_probe(struct platform_device *pdev)
if (ret)
return ret;
- /*
- * BD70528 irq controller is not touching the main mask register.
- * So enable the RTC block interrupts at main level. We can just
- * leave them enabled as irq-controller should disable irqs
- * from sub-registers when IRQ is disabled or freed.
- */
- if (enable_main_irq) {
- ret = regmap_update_bits(bd_rtc->regmap,
- BD70528_REG_INT_MAIN_MASK,
- BD70528_INT_RTC_MASK, 0);
- if (ret) {
- dev_err(&pdev->dev, "Failed to enable RTC interrupts\n");
- return ret;
- }
- }
-
return devm_rtc_register_device(rtc);
}
static const struct platform_device_id bd718x7_rtc_id[] = {
- { "bd70528-rtc", ROHM_CHIP_TYPE_BD70528 },
{ "bd71828-rtc", ROHM_CHIP_TYPE_BD71828 },
{ "bd71815-rtc", ROHM_CHIP_TYPE_BD71815 },
{ },
@@ -649,6 +355,6 @@ static struct platform_driver bd70528_rtc = {
module_platform_driver(bd70528_rtc);
MODULE_AUTHOR("Matti Vaittinen <[email protected]>");
-MODULE_DESCRIPTION("ROHM BD70528 and BD71828 PMIC RTC driver");
+MODULE_DESCRIPTION("ROHM BD71828 and BD71815 PMIC RTC driver");
MODULE_LICENSE("GPL");
MODULE_ALIAS("platform:bd70528-rtc");
--
2.25.4
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
On Tue, 2021-05-25 at 13:47 +0200, Alexandre Belloni wrote:
> On 25/05/2021 13:14:09+0300, Matti Vaittinen wrote:
> > The only known BD70528 use-cases are such that the PMIC is
> > controlled
> > from separate MCU which is not running Linux. I am not aware of
> > any Linux driver users. Furthermore, it seems there is no demand
> > for
> > this IC. Let's ease the maintenance burden and drop the driver. We
> > can
> > always add it back if there is sudden need for it.
> >
> > Signed-off-by: Matti Vaittinen <[email protected]>
> >
> > ---
> > My heart is a bit less bleeding when I see how much simpler this
> > RTC
> > driver became..
> > Please let me know if some of you think the driver is needed.
> > ---
> > drivers/rtc/Kconfig | 4 +-
> > drivers/rtc/rtc-bd70528.c | 316 ++------------------------------
> > ------
> > 2 files changed, 13 insertions(+), 307 deletions(-)
> >
> > diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> > index d8c13fded164..398899217626 100644
> > --- a/drivers/rtc/Kconfig
> > +++ b/drivers/rtc/Kconfig
> > @@ -502,10 +502,10 @@ config RTC_DRV_M41T80_WDT
> >
> > config RTC_DRV_BD70528
> > tristate "ROHM BD70528, BD71815 and BD71828 PMIC RTC"
> > - depends on MFD_ROHM_BD71828 || MFD_ROHM_BD70528 &&
> > (BD70528_WATCHDOG || !BD70528_WATCHDOG)
> > + depends on MFD_ROHM_BD71828
>
> This won't apply as I applied b0ddc5b17005 ("rtc: bd70528: fix
> BD71815
> watchdog dependency") yesterday...
>
I know Alexandre. Sorry about that. I received confirmation to my
question whether the BD70528 is used only at this morning. Sorry for
all the work.
This is why the cover-letter stated:
"...As a final note - Few improvements/fixes were just applied to the
regulator tree so this series is likely to conflict. Some fixes
were also added to RTC Kconfig - which means also the RTC tree
may have conflicts. Please let me know if you wish me to rebase
this series or those patches."
And in any case applying the MFD and RTC patches should be synced. RTC
parts should be applied before MFD parts because few other PMICs use
this same RTC driver and compilation would fail with missing headers if
MFD was removed before RTC changes are applied.
Suggestions on how to guarantee the order of MFD and RTC - and how to
resolve conflicts?
Best Regards
Matti Vaittinen
On 25/05/2021 13:14:09+0300, Matti Vaittinen wrote:
> The only known BD70528 use-cases are such that the PMIC is controlled
> from separate MCU which is not running Linux. I am not aware of
> any Linux driver users. Furthermore, it seems there is no demand for
> this IC. Let's ease the maintenance burden and drop the driver. We can
> always add it back if there is sudden need for it.
>
> Signed-off-by: Matti Vaittinen <[email protected]>
>
> ---
> My heart is a bit less bleeding when I see how much simpler this RTC
> driver became..
> Please let me know if some of you think the driver is needed.
> ---
> drivers/rtc/Kconfig | 4 +-
> drivers/rtc/rtc-bd70528.c | 316 ++------------------------------------
> 2 files changed, 13 insertions(+), 307 deletions(-)
>
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index d8c13fded164..398899217626 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -502,10 +502,10 @@ config RTC_DRV_M41T80_WDT
>
> config RTC_DRV_BD70528
> tristate "ROHM BD70528, BD71815 and BD71828 PMIC RTC"
> - depends on MFD_ROHM_BD71828 || MFD_ROHM_BD70528 && (BD70528_WATCHDOG || !BD70528_WATCHDOG)
> + depends on MFD_ROHM_BD71828
This won't apply as I applied b0ddc5b17005 ("rtc: bd70528: fix BD71815
watchdog dependency") yesterday...
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On 25/05/2021 11:59:58+0000, Vaittinen, Matti wrote:
> And in any case applying the MFD and RTC patches should be synced. RTC
> parts should be applied before MFD parts because few other PMICs use
> this same RTC driver and compilation would fail with missing headers if
> MFD was removed before RTC changes are applied.
>
> Suggestions on how to guarantee the order of MFD and RTC - and how to
> resolve conflicts?
>
The easiest is to take the RTC patches in one cycle and the rest in
the next cycle.
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On Tue, 2021-05-25 at 15:08 +0200, Alexandre Belloni wrote:
> On 25/05/2021 11:59:58+0000, Vaittinen, Matti wrote:
> > And in any case applying the MFD and RTC patches should be synced.
> > RTC
> > parts should be applied before MFD parts because few other PMICs
> > use
> > this same RTC driver and compilation would fail with missing
> > headers if
> > MFD was removed before RTC changes are applied.
> >
> > Suggestions on how to guarantee the order of MFD and RTC - and how
> > to
> > resolve conflicts?
> >
>
> The easiest is to take the RTC patches in one cycle and the rest in
> the next cycle.
>
>
That suits me just fine. Should I resend the series w/o MFD changes and
send the MFD part only at next cycle?
Best Regards
Matti Vaittinen