| Message ID | 20190703164822.17924-1-frank-w@public-files.de |
|---|---|
| Headers | show |
| Series | implement poweroff for mt6323/6397 | expand |
On 03/07/2019 18:48, Frank Wunderlich wrote: > From: Josef Friedl <josef.friedl@speed.at> > Still missing commit message. Describe here why you need to do that. > Suggested-by: Frank Wunderlich <frank-w@public-files.de> > Signed-off-by: Josef Friedl <josef.friedl@speed.at> > Signed-off-by: Frank Wunderlich <frank-w@public-files.de> > =2D-- Please check your email setting as discussed offline. Otherwise your patches won't get accepted. Regards, Matthias > drivers/rtc/rtc-mt6397.c | 55 +------------------------- > include/linux/mfd/mt6397/rtc.h | 71 ++++++++++++++++++++++++++++++++++ > 2 files changed, 72 insertions(+), 54 deletions(-) > create mode 100644 include/linux/mfd/mt6397/rtc.h > > diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c > index b46ed4dc7015..c08ee5edf865 100644 > =2D-- a/drivers/rtc/rtc-mt6397.c > +++ b/drivers/rtc/rtc-mt6397.c > @@ -9,60 +9,7 @@ > #include <linux/module.h> > #include <linux/regmap.h> > #include <linux/rtc.h> > -#include <linux/irqdomain.h> > -#include <linux/platform_device.h> > -#include <linux/of_address.h> > -#include <linux/of_irq.h> > -#include <linux/io.h> > -#include <linux/mfd/mt6397/core.h> > - > -#define RTC_BBPU 0x0000 > -#define RTC_BBPU_CBUSY BIT(6) > - > -#define RTC_WRTGR 0x003c > - > -#define RTC_IRQ_STA 0x0002 > -#define RTC_IRQ_STA_AL BIT(0) > -#define RTC_IRQ_STA_LP BIT(3) > - > -#define RTC_IRQ_EN 0x0004 > -#define RTC_IRQ_EN_AL BIT(0) > -#define RTC_IRQ_EN_ONESHOT BIT(2) > -#define RTC_IRQ_EN_LP BIT(3) > -#define RTC_IRQ_EN_ONESHOT_AL (RTC_IRQ_EN_ONESHOT | RTC_IRQ_EN_AL) > - > -#define RTC_AL_MASK 0x0008 > -#define RTC_AL_MASK_DOW BIT(4) > - > -#define RTC_TC_SEC 0x000a > -/* Min, Hour, Dom... register offset to RTC_TC_SEC */ > -#define RTC_OFFSET_SEC 0 > -#define RTC_OFFSET_MIN 1 > -#define RTC_OFFSET_HOUR 2 > -#define RTC_OFFSET_DOM 3 > -#define RTC_OFFSET_DOW 4 > -#define RTC_OFFSET_MTH 5 > -#define RTC_OFFSET_YEAR 6 > -#define RTC_OFFSET_COUNT 7 > - > -#define RTC_AL_SEC 0x0018 > - > -#define RTC_PDN2 0x002e > -#define RTC_PDN2_PWRON_ALARM BIT(4) > - > -#define RTC_MIN_YEAR 1968 > -#define RTC_BASE_YEAR 1900 > -#define RTC_NUM_YEARS 128 > -#define RTC_MIN_YEAR_OFFSET (RTC_MIN_YEAR - RTC_BASE_YEAR) > - > -struct mt6397_rtc { > - struct device *dev; > - struct rtc_device *rtc_dev; > - struct mutex lock; > - struct regmap *regmap; > - int irq; > - u32 addr_base; > -}; > +#include <linux/mfd/mt6397/rtc.h> > > static int mtk_rtc_write_trigger(struct mt6397_rtc *rtc) > { > diff --git a/include/linux/mfd/mt6397/rtc.h b/include/linux/mfd/mt6397/rtc= > .h > new file mode 100644 > index 000000000000..b702c29e8c74 > =2D-- /dev/null > +++ b/include/linux/mfd/mt6397/rtc.h > @@ -0,0 +1,71 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (C) 2014-2018 MediaTek Inc. > + * > + * Author: Tianping.Fang <tianping.fang@mediatek.com> > + * Sean Wang <sean.wang@mediatek.com> > + */ > + > +#ifndef _LINUX_MFD_MT6397_RTC_H_ > +#define _LINUX_MFD_MT6397_RTC_H_ > + > +#include <linux/jiffies.h> > +#include <linux/mutex.h> > +#include <linux/regmap.h> > +#include <linux/rtc.h> > + > +#define RTC_BBPU 0x0000 > +#define RTC_BBPU_CBUSY BIT(6) > +#define RTC_BBPU_KEY (0x43 << 8) > + > +#define RTC_WRTGR 0x003c > + > +#define RTC_IRQ_STA 0x0002 > +#define RTC_IRQ_STA_AL BIT(0) > +#define RTC_IRQ_STA_LP BIT(3) > + > +#define RTC_IRQ_EN 0x0004 > +#define RTC_IRQ_EN_AL BIT(0) > +#define RTC_IRQ_EN_ONESHOT BIT(2) > +#define RTC_IRQ_EN_LP BIT(3) > +#define RTC_IRQ_EN_ONESHOT_AL (RTC_IRQ_EN_ONESHOT | RTC_IRQ_EN_AL) > + > +#define RTC_AL_MASK 0x0008 > +#define RTC_AL_MASK_DOW BIT(4) > + > +#define RTC_TC_SEC 0x000a > +/* Min, Hour, Dom... register offset to RTC_TC_SEC */ > +#define RTC_OFFSET_SEC 0 > +#define RTC_OFFSET_MIN 1 > +#define RTC_OFFSET_HOUR 2 > +#define RTC_OFFSET_DOM 3 > +#define RTC_OFFSET_DOW 4 > +#define RTC_OFFSET_MTH 5 > +#define RTC_OFFSET_YEAR 6 > +#define RTC_OFFSET_COUNT 7 > + > +#define RTC_AL_SEC 0x0018 > + > +#define RTC_PDN2 0x002e > +#define RTC_PDN2_PWRON_ALARM BIT(4) > + > +#define RTC_MIN_YEAR 1968 > +#define RTC_BASE_YEAR 1900 > +#define RTC_NUM_YEARS 128 > +#define RTC_MIN_YEAR_OFFSET (RTC_MIN_YEAR - RTC_BASE_YEAR) > + > +#define MTK_RTC_POLL_DELAY_US 10 > +#define MTK_RTC_POLL_TIMEOUT (jiffies_to_usecs(HZ)) > + > +struct mt6397_rtc { > + struct device *dev; > + struct rtc_device *rtc_dev; > + > + /* Protect register access from multiple tasks */ > + struct mutex lock; > + struct regmap *regmap; > + int irq; > + u32 addr_base; > +}; > + > +#endif /* _LINUX_MFD_MT6397_RTC_H_ */ > =2D- > 2.17.1 >
On 03/07/2019 18:48, Frank Wunderlich wrote: > From: Josef Friedl <josef.friedl@speed.at> > > - use regmap_read_poll_timeout to drop while-loop > - use devm-api to drop remove-callback > - add new compatible for mt6323 > It's up to the maintainer but I don't like patches doing clean-ups together with adding support for new HW, although it's a trivial one here. > Signed-off-by: Frank Wunderlich <frank-w@public-files.de> > =2D-- > drivers/rtc/rtc-mt6397.c | 55 ++++++++++++++++------------------------ > 1 file changed, 22 insertions(+), 33 deletions(-) > > diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c > index c08ee5edf865..e5ddf0d0b6f1 100644 > =2D-- a/drivers/rtc/rtc-mt6397.c > +++ b/drivers/rtc/rtc-mt6397.c > @@ -4,16 +4,19 @@ > * Author: Tianping.Fang <tianping.fang@mediatek.com> Missing in the CC list.
On 03/07/2019 18:48, Frank Wunderlich wrote: > From: Josef Friedl <josef.friedl@speed.at> > > Suggested-by: Frank Wunderlich <frank-w@public-files.de> > Signed-off-by: Josef Friedl <josef.friedl@speed.at> Why is there a Signed-off-by from Josef for this patch but not for the others?
On Wed, 2019-07-03 at 18:48 +0200, Frank Wunderlich wrote: > From: Josef Friedl <josef.friedl@speed.at> > > Suggested-by: Frank Wunderlich <frank-w@public-files.de> > Signed-off-by: Josef Friedl <josef.friedl@speed.at> > Signed-off-by: Frank Wunderlich <frank-w@public-files.de> > --- > drivers/power/reset/Kconfig | 10 +++ > drivers/power/reset/Makefile | 1 + > drivers/power/reset/mt6323-poweroff.c | 97 +++++++++++++++++++++++++++ > include/linux/mfd/mt6397/core.h | 2 + > 4 files changed, 110 insertions(+) > create mode 100644 drivers/power/reset/mt6323-poweroff.c > > -- > 2.17.1 > > diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig > index 980951dff834..492678e22088 100644 > --- a/drivers/power/reset/Kconfig > +++ b/drivers/power/reset/Kconfig > @@ -140,6 +140,16 @@ config POWER_RESET_LTC2952 > This driver supports an external powerdown trigger and board power > down via the LTC2952. Bindings are made in the device tree. > > +config POWER_RESET_MT6323 > + bool "MediaTek MT6323 power-off driver" > + depends on MFD_MT6397 > + help > + The power-off driver is responsible for externally shutdown down > + the power of a remote MediaTek SoC MT6323 is connected to through > + controlling a tiny circuit BBPU inside MT6323 RTC. > + > + Say Y if you have a board where MT6323 could be found. > + > config POWER_RESET_QNAP > bool "QNAP power-off driver" > depends on OF_GPIO && PLAT_ORION > diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile > index 0aebee954ac1..94eaceb01d66 100644 > --- a/drivers/power/reset/Makefile > +++ b/drivers/power/reset/Makefile > @@ -11,6 +11,7 @@ obj-$(CONFIG_POWER_RESET_GPIO) += gpio-poweroff.o > obj-$(CONFIG_POWER_RESET_GPIO_RESTART) += gpio-restart.o > obj-$(CONFIG_POWER_RESET_HISI) += hisi-reboot.o > obj-$(CONFIG_POWER_RESET_MSM) += msm-poweroff.o > +obj-$(CONFIG_POWER_RESET_MT6323) += mt6323-poweroff.o > obj-$(CONFIG_POWER_RESET_QCOM_PON) += qcom-pon.o > obj-$(CONFIG_POWER_RESET_OCELOT_RESET) += ocelot-reset.o > obj-$(CONFIG_POWER_RESET_PIIX4_POWEROFF) += piix4-poweroff.o > diff --git a/drivers/power/reset/mt6323-poweroff.c b/drivers/power/reset/mt6323-poweroff.c > new file mode 100644 > index 000000000000..1caf43d9e46d > --- /dev/null > +++ b/drivers/power/reset/mt6323-poweroff.c > @@ -0,0 +1,97 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Power off through MediaTek PMIC > + * > + * Copyright (C) 2018 MediaTek Inc. > + * > + * Author: Sean Wang <sean.wang@mediatek.com> > + * > + */ > + > +#include <linux/err.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/platform_device.h> > +#include <linux/mfd/mt6397/core.h> > +#include <linux/mfd/mt6397/rtc.h> > + > +struct mt6323_pwrc { > + struct device *dev; > + struct regmap *regmap; > + u32 base; > +}; > + > +static struct mt6323_pwrc *mt_pwrc; > + > +static void mt6323_do_pwroff(void) > +{ > + struct mt6323_pwrc *pwrc = mt_pwrc; > + unsigned int val; > + int ret; > + > + regmap_write(pwrc->regmap, pwrc->base + RTC_BBPU, RTC_BBPU_KEY); > + regmap_write(pwrc->regmap, pwrc->base + RTC_WRTGR, 1); > + > + ret = regmap_read_poll_timeout(pwrc->regmap, > + pwrc->base + RTC_BBPU, val, > + !(val & RTC_BBPU_CBUSY), > + MTK_RTC_POLL_DELAY_US, > + MTK_RTC_POLL_TIMEOUT); > + if (ret) > + dev_err(pwrc->dev, "failed to write BBPU: %d\n", ret); > + > + /* Wait some time until system down, otherwise, notice with a warn */ > + mdelay(1000); > + > + WARN_ONCE(1, "Unable to power off system\n"); > +} > + > +static int mt6323_pwrc_probe(struct platform_device *pdev) > +{ > + struct mt6397_chip *mt6397_chip = dev_get_drvdata(pdev->dev.parent); > + struct mt6323_pwrc *pwrc; > + struct resource *res; > + > + pwrc = devm_kzalloc(&pdev->dev, sizeof(*pwrc), GFP_KERNEL); > + if (!pwrc) > + return -ENOMEM; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + pwrc->base = res->start; > + pwrc->regmap = mt6397_chip->regmap; > + pwrc->dev = &pdev->dev; > + mt_pwrc = pwrc; > + > + pm_power_off = &mt6323_do_pwroff; We had implement MT8173 poweroff function in arm-trusted-firmware's PSCI plat_system_off() function. MT8173 SoC is using PMIC MT6397. (Ref: https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/mediatek/mt8173/plat_pm.c and https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/mediatek/mt8173/drivers/rtc) Do you think it's better to implement poweroff function into arm-trusted-firmware compared to hijack pm_poweroff() function in Kernel? Right now, we are doing the upstream of other PMIC chip like MT6358's poweroff function in arm-trusted-firmware too. > + > + return 0; > +} > + > +static int mt6323_pwrc_remove(struct platform_device *pdev) > +{ > + if (pm_power_off == &mt6323_do_pwroff) > + pm_power_off = NULL; > + > + return 0; > +} > + > +static const struct of_device_id mt6323_pwrc_dt_match[] = { > + { .compatible = "mediatek,mt6323-pwrc" }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, mt6323_pwrc_dt_match); > + > +static struct platform_driver mt6323_pwrc_driver = { > + .probe = mt6323_pwrc_probe, > + .remove = mt6323_pwrc_remove, > + .driver = { > + .name = "mt6323-pwrc", > + .of_match_table = mt6323_pwrc_dt_match, > + }, > +}; > + > +module_platform_driver(mt6323_pwrc_driver); > + > +MODULE_DESCRIPTION("Poweroff driver for MT6323 PMIC"); > +MODULE_AUTHOR("Sean Wang <sean.wang@mediatek.com>"); > +MODULE_LICENSE("GPL v2"); > diff --git a/include/linux/mfd/mt6397/core.h b/include/linux/mfd/mt6397/core.h > index 25a95e72179b..652da61e3711 100644 > --- a/include/linux/mfd/mt6397/core.h > +++ b/include/linux/mfd/mt6397/core.h > @@ -7,6 +7,8 @@ > #ifndef __MFD_MT6397_CORE_H__ > #define __MFD_MT6397_CORE_H__ > > +#include <linux/mutex.h> > + > enum mt6397_irq_numbers { > MT6397_IRQ_SPKL_AB = 0,
> Gesendet: Donnerstag, 04. Juli 2019 um 12:03 Uhr > Von: "Ran Bi" <ran.bi@mediatek.com> > We had implement MT8173 poweroff function in arm-trusted-firmware's PSCI > plat_system_off() function. MT8173 SoC is using PMIC MT6397. (Ref: > https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/mediatek/mt8173/plat_pm.c and https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/mediatek/mt8173/drivers/rtc) Do you think it's better to implement poweroff function into arm-trusted-firmware compared to hijack pm_poweroff() function in Kernel? Right now, we are doing the upstream of other PMIC chip like MT6358's poweroff function in arm-trusted-firmware too. ATF imho only used for arm64, my board is 32bit armv7 and i (currently) do not boot up with ATF regards Frank
> Gesendet: Donnerstag, 04. Juli 2019 um 11:13 Uhr > Von: "Matthias Brugger" <matthias.bgg@gmail.com> > It's up to the maintainer but I don't like patches doing clean-ups together with > adding support for new HW, although it's a trivial one here. i can split again to have clean-up and new functions separated
> Still missing commit message. Describe here why you need to do that. ok, added note that headers are reused in power-off-driver https://github.com/frank-w/BPI-R2-4.14/commits/5.2-poweroff-mainline > Please check your email setting as discussed offline. Otherwise your patches > won't get accepted. tested with webmailer where it looks good :( seems the problem is only shown when imported to patchwork using only git sendemail in ubuntu 18.4 without any mta (have sendmail not installed) and no changes made to git sendemail except authentication. i see that (except cover-letter which is quoted-printable) all is send with Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit so i have forced git sendemail now to sendemail.composeencoding UTF-8 if this does not work i can try instead sendemail.transferEncoding 8bit regards Frank
On 03/07/2019 18:48:18+0200, Frank Wunderlich wrote: > @@ -271,14 +268,11 @@ static int mtk_rtc_probe(struct platform_device *pdev) > > platform_set_drvdata(pdev, rtc); > > - rtc->rtc_dev = devm_rtc_allocate_device(rtc->dev); > - if (IS_ERR(rtc->rtc_dev)) > - return PTR_ERR(rtc->rtc_dev); > + ret = devm_request_threaded_irq(&pdev->dev, rtc->irq, NULL, > + mtk_rtc_irq_handler_thread, > + IRQF_ONESHOT | IRQF_TRIGGER_HIGH, > + "mt6397-rtc", rtc); > This change may lead to a crash and the allocation was intentionally placed before the irq request. > - ret = request_threaded_irq(rtc->irq, NULL, > - mtk_rtc_irq_handler_thread, > - IRQF_ONESHOT | IRQF_TRIGGER_HIGH, > - "mt6397-rtc", rtc); > if (ret) { > dev_err(&pdev->dev, "Failed to request alarm IRQ: %d: %d\n", > rtc->irq, ret); > @@ -287,6 +281,10 @@ static int mtk_rtc_probe(struct platform_device *pdev) > > device_init_wakeup(&pdev->dev, 1); > > + rtc->rtc_dev = devm_rtc_allocate_device(&pdev->dev); > + if (IS_ERR(rtc->rtc_dev)) > + return PTR_ERR(rtc->rtc_dev); > + > rtc->rtc_dev->ops = &mtk_rtc_ops; > > ret = rtc_register_device(rtc->rtc_dev); > @@ -302,15 +300,6 @@ static int mtk_rtc_probe(struct platform_device *pdev) > return ret; > } > > -static int mtk_rtc_remove(struct platform_device *pdev) > -{ > - struct mt6397_rtc *rtc = platform_get_drvdata(pdev); > - > - free_irq(rtc->irq, rtc); > - > - return 0; > -} > - > #ifdef CONFIG_PM_SLEEP > static int mt6397_rtc_suspend(struct device *dev) > { > @@ -337,6 +326,7 @@ static SIMPLE_DEV_PM_OPS(mt6397_pm_ops, mt6397_rtc_suspend, > mt6397_rtc_resume); > > static const struct of_device_id mt6397_rtc_of_match[] = { > + { .compatible = "mediatek,mt6323-rtc", }, Unrelated change, this is not an improvement and must be accompanied by a documentation change. > { .compatible = "mediatek,mt6397-rtc", }, > { } > }; > @@ -349,7 +339,6 @@ static struct platform_driver mtk_rtc_driver = { > .pm = &mt6397_pm_ops, > }, > .probe = mtk_rtc_probe, > - .remove = mtk_rtc_remove, > }; > > module_platform_driver(mtk_rtc_driver); > -- > 2.17.1 >
Hi Alexander, thank you for the Review > Gesendet: Donnerstag, 04. Juli 2019 um 22:43 Uhr > Von: "Alexandre Belloni" <alexandre.belloni@bootlin.com> > > - rtc->rtc_dev = devm_rtc_allocate_device(rtc->dev); > > - if (IS_ERR(rtc->rtc_dev)) > > - return PTR_ERR(rtc->rtc_dev); > > + ret = devm_request_threaded_irq(&pdev->dev, rtc->irq, NULL, > > + mtk_rtc_irq_handler_thread, > > + IRQF_ONESHOT | IRQF_TRIGGER_HIGH, > > + "mt6397-rtc", rtc); > > > > This change may lead to a crash and the allocation was intentionally > placed before the irq request. i got no crash till now, but i will try to move the allocation before irq-request > > - ret = request_threaded_irq(rtc->irq, NULL, > > - mtk_rtc_irq_handler_thread, > > - IRQF_ONESHOT | IRQF_TRIGGER_HIGH, > > - "mt6397-rtc", rtc); > > if (ret) { > > dev_err(&pdev->dev, "Failed to request alarm IRQ: %d: %d\n", > > rtc->irq, ret); > > @@ -287,6 +281,10 @@ static int mtk_rtc_probe(struct platform_device *pdev) > > > > device_init_wakeup(&pdev->dev, 1); > > > > + rtc->rtc_dev = devm_rtc_allocate_device(&pdev->dev); > > + if (IS_ERR(rtc->rtc_dev)) > > + return PTR_ERR(rtc->rtc_dev); > > + > > rtc->rtc_dev->ops = &mtk_rtc_ops; > > static const struct of_device_id mt6397_rtc_of_match[] = { > > + { .compatible = "mediatek,mt6323-rtc", }, > > Unrelated change, this is not an improvement and must be accompanied by > a documentation change. documentation is changed in 1/7 defining this compatible. i called it improvement because existing driver now supports another chip regards Frank
On 05/07/2019 17:35:46+0200, Frank Wunderlich wrote: > Hi Alexander, > > thank you for the Review > > > Gesendet: Donnerstag, 04. Juli 2019 um 22:43 Uhr > > Von: "Alexandre Belloni" <alexandre.belloni@bootlin.com> > > > - rtc->rtc_dev = devm_rtc_allocate_device(rtc->dev); > > > - if (IS_ERR(rtc->rtc_dev)) > > > - return PTR_ERR(rtc->rtc_dev); > > > + ret = devm_request_threaded_irq(&pdev->dev, rtc->irq, NULL, > > > + mtk_rtc_irq_handler_thread, > > > + IRQF_ONESHOT | IRQF_TRIGGER_HIGH, > > > + "mt6397-rtc", rtc); > > > > > > > This change may lead to a crash and the allocation was intentionally > > placed before the irq request. > > i got no crash till now, but i will try to move the allocation before irq-request > Let's say the RTC has been used to start your platform, then the irq handler will be called as soon as the irq is requested, leading to a null pointer dereference. > > > - ret = request_threaded_irq(rtc->irq, NULL, > > > - mtk_rtc_irq_handler_thread, > > > - IRQF_ONESHOT | IRQF_TRIGGER_HIGH, > > > - "mt6397-rtc", rtc); > > > if (ret) { > > > dev_err(&pdev->dev, "Failed to request alarm IRQ: %d: %d\n", > > > rtc->irq, ret); > > > @@ -287,6 +281,10 @@ static int mtk_rtc_probe(struct platform_device *pdev) > > > > > > device_init_wakeup(&pdev->dev, 1); > > > > > > + rtc->rtc_dev = devm_rtc_allocate_device(&pdev->dev); > > > + if (IS_ERR(rtc->rtc_dev)) > > > + return PTR_ERR(rtc->rtc_dev); > > > + > > > rtc->rtc_dev->ops = &mtk_rtc_ops; > > > > > static const struct of_device_id mt6397_rtc_of_match[] = { > > > + { .compatible = "mediatek,mt6323-rtc", }, > > > > Unrelated change, this is not an improvement and must be accompanied by > > a documentation change. > > documentation is changed in 1/7 defining this compatible. i called it improvement because existing driver now supports another chip > Yes and IIRC, I did comment that the rtc change also had to be separated from 1/7. Also, I really doubt this new compatible is necessary at all as you could simply directly use mediatek,mt6397-rtc.
> Gesendet: Freitag, 05. Juli 2019 um 23:24 Uhr > Von: "Alexandre Belloni" <alexandre.belloni@bootlin.com> > Let's say the RTC has been used to start your platform, then the irq > handler will be called as soon as the irq is requested, leading to a > null pointer dereference. i cannot test this with my platform, but i have changed it in my repo https://github.com/frank-w/BPI-R2-4.14/commits/5.2-poweroff-mainline > Yes and IIRC, I did comment that the rtc change also had to be separated > from 1/7. also this is put in separate commit, can you take a look before i post v3? > Also, I really doubt this new compatible is necessary at all as you > could simply directly use mediatek,mt6397-rtc. imho this can confuse because the wrong chip-name is used in dts regards Frank
On 06/07/2019 18:15:20+0200, Frank Wunderlich wrote: > > Gesendet: Freitag, 05. Juli 2019 um 23:24 Uhr > > Von: "Alexandre Belloni" <alexandre.belloni@bootlin.com> > > > Let's say the RTC has been used to start your platform, then the irq > > handler will be called as soon as the irq is requested, leading to a > > null pointer dereference. > > i cannot test this with my platform, but i have changed it in my repo > > https://github.com/frank-w/BPI-R2-4.14/commits/5.2-poweroff-mainline > > > Yes and IIRC, I did comment that the rtc change also had to be separated > > from 1/7. > > also this is put in separate commit, can you take a look before i post v3? > > > Also, I really doubt this new compatible is necessary at all as you > > could simply directly use mediatek,mt6397-rtc. > > imho this can confuse because the wrong chip-name is used in dts > This is not true, we do that all the time and the immediate benefit of using the mt6397 compatible is that then there is no need to synchronize between subsystems. If you want to be absolutely conservative, you could use compatible = "mediatek,mt6323-rtc", "mediatek,mt6397-rtc"; in your DT.