* [PATCH] rtc: remove cast to void pointer
From: Himanshu Jha @ 2017-08-29 14:39 UTC (permalink / raw)
To: alexandre.belloni
Cc: a.zummo, patrice.chotard, mcoquelin.stm32, maxime.ripard,
michal.simek, linux-rtc, linux-kernel, linux-arm-kernel,
Himanshu Jha
casting to void pointer from any pointer type and vice-versa is done
implicitly and therefore casting is not needed in such a case.
Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
---
drivers/rtc/interface.c | 4 ++--
drivers/rtc/rtc-88pm80x.c | 2 +-
drivers/rtc/rtc-88pm860x.c | 2 +-
drivers/rtc/rtc-at32ap700x.c | 2 +-
drivers/rtc/rtc-hym8563.c | 2 +-
drivers/rtc/rtc-m48t59.c | 2 +-
drivers/rtc/rtc-max8925.c | 2 +-
drivers/rtc/rtc-pic32.c | 2 +-
drivers/rtc/rtc-s3c.c | 4 ++--
drivers/rtc/rtc-st-lpc.c | 2 +-
drivers/rtc/rtc-stm32.c | 2 +-
drivers/rtc/rtc-sun6i.c | 2 +-
drivers/rtc/rtc-sunxi.c | 2 +-
drivers/rtc/rtc-vr41xx.c | 4 ++--
drivers/rtc/rtc-xgene.c | 2 +-
drivers/rtc/rtc-zynqmp.c | 2 +-
16 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
index 8cec9a0..81c85d2 100644
--- a/drivers/rtc/interface.c
+++ b/drivers/rtc/interface.c
@@ -528,7 +528,7 @@ void rtc_handle_legacy_irq(struct rtc_device *rtc, int num, int mode)
*/
void rtc_aie_update_irq(void *private)
{
- struct rtc_device *rtc = (struct rtc_device *)private;
+ struct rtc_device *rtc = private;
rtc_handle_legacy_irq(rtc, 1, RTC_AF);
}
@@ -541,7 +541,7 @@ void rtc_aie_update_irq(void *private)
*/
void rtc_uie_update_irq(void *private)
{
- struct rtc_device *rtc = (struct rtc_device *)private;
+ struct rtc_device *rtc = private;
rtc_handle_legacy_irq(rtc, 1, RTC_UF);
}
diff --git a/drivers/rtc/rtc-88pm80x.c b/drivers/rtc/rtc-88pm80x.c
index 466bf7f..819612e 100644
--- a/drivers/rtc/rtc-88pm80x.c
+++ b/drivers/rtc/rtc-88pm80x.c
@@ -60,7 +60,7 @@ struct pm80x_rtc_info {
static irqreturn_t rtc_update_handler(int irq, void *data)
{
- struct pm80x_rtc_info *info = (struct pm80x_rtc_info *)data;
+ struct pm80x_rtc_info *info = data;
int mask;
mask = PM800_ALARM | PM800_ALARM_WAKEUP;
diff --git a/drivers/rtc/rtc-88pm860x.c b/drivers/rtc/rtc-88pm860x.c
index 19e53b3..67f7fa3 100644
--- a/drivers/rtc/rtc-88pm860x.c
+++ b/drivers/rtc/rtc-88pm860x.c
@@ -59,7 +59,7 @@ struct pm860x_rtc_info {
static irqreturn_t rtc_update_handler(int irq, void *data)
{
- struct pm860x_rtc_info *info = (struct pm860x_rtc_info *)data;
+ struct pm860x_rtc_info *info = data;
int mask;
mask = ALARM | ALARM_WAKEUP;
diff --git a/drivers/rtc/rtc-at32ap700x.c b/drivers/rtc/rtc-at32ap700x.c
index de8bf56..f603f99 100644
--- a/drivers/rtc/rtc-at32ap700x.c
+++ b/drivers/rtc/rtc-at32ap700x.c
@@ -164,7 +164,7 @@ static int at32_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
static irqreturn_t at32_rtc_interrupt(int irq, void *dev_id)
{
- struct rtc_at32ap700x *rtc = (struct rtc_at32ap700x *)dev_id;
+ struct rtc_at32ap700x *rtc = dev_id;
unsigned long isr = rtc_readl(rtc, ISR);
unsigned long events = 0;
int ret = IRQ_NONE;
diff --git a/drivers/rtc/rtc-hym8563.c b/drivers/rtc/rtc-hym8563.c
index e5ad527..0709968 100644
--- a/drivers/rtc/rtc-hym8563.c
+++ b/drivers/rtc/rtc-hym8563.c
@@ -436,7 +436,7 @@ static struct clk *hym8563_clkout_register_clk(struct hym8563 *hym8563)
*/
static irqreturn_t hym8563_irq(int irq, void *dev_id)
{
- struct hym8563 *hym8563 = (struct hym8563 *)dev_id;
+ struct hym8563 *hym8563 = dev_id;
struct i2c_client *client = hym8563->client;
struct mutex *lock = &hym8563->rtc->ops_lock;
int data, ret;
diff --git a/drivers/rtc/rtc-m48t59.c b/drivers/rtc/rtc-m48t59.c
index d99a705..2e35f73 100644
--- a/drivers/rtc/rtc-m48t59.c
+++ b/drivers/rtc/rtc-m48t59.c
@@ -302,7 +302,7 @@ static int m48t59_rtc_proc(struct device *dev, struct seq_file *seq)
*/
static irqreturn_t m48t59_rtc_interrupt(int irq, void *dev_id)
{
- struct device *dev = (struct device *)dev_id;
+ struct device *dev = dev_id;
struct platform_device *pdev = to_platform_device(dev);
struct m48t59_plat_data *pdata = dev_get_platdata(&pdev->dev);
struct m48t59_private *m48t59 = platform_get_drvdata(pdev);
diff --git a/drivers/rtc/rtc-max8925.c b/drivers/rtc/rtc-max8925.c
index 67d6fc2..45d7e40 100644
--- a/drivers/rtc/rtc-max8925.c
+++ b/drivers/rtc/rtc-max8925.c
@@ -74,7 +74,7 @@ struct max8925_rtc_info {
static irqreturn_t rtc_update_handler(int irq, void *data)
{
- struct max8925_rtc_info *info = (struct max8925_rtc_info *)data;
+ struct max8925_rtc_info *info = data;
/* disable ALARM0 except for 1SEC alarm */
max8925_set_bits(info->rtc, MAX8925_ALARM0_CNTL, 0x7f, 0);
diff --git a/drivers/rtc/rtc-pic32.c b/drivers/rtc/rtc-pic32.c
index 5cfb6df..04788c3 100644
--- a/drivers/rtc/rtc-pic32.c
+++ b/drivers/rtc/rtc-pic32.c
@@ -92,7 +92,7 @@ static void pic32_rtc_alarm_clk_enable(struct pic32_rtc_dev *pdata,
static irqreturn_t pic32_rtc_alarmirq(int irq, void *id)
{
- struct pic32_rtc_dev *pdata = (struct pic32_rtc_dev *)id;
+ struct pic32_rtc_dev *pdata = id;
clk_enable(pdata->clk);
rtc_update_irq(pdata->rtc, 1, RTC_AF | RTC_IRQF);
diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
index a8992c2..8e8a815 100644
--- a/drivers/rtc/rtc-s3c.c
+++ b/drivers/rtc/rtc-s3c.c
@@ -113,7 +113,7 @@ static void s3c_rtc_disable_clk(struct s3c_rtc *info)
/* IRQ Handlers */
static irqreturn_t s3c_rtc_tickirq(int irq, void *id)
{
- struct s3c_rtc *info = (struct s3c_rtc *)id;
+ struct s3c_rtc *info = id;
if (info->data->irq_handler)
info->data->irq_handler(info, S3C2410_INTP_TIC);
@@ -123,7 +123,7 @@ static irqreturn_t s3c_rtc_tickirq(int irq, void *id)
static irqreturn_t s3c_rtc_alarmirq(int irq, void *id)
{
- struct s3c_rtc *info = (struct s3c_rtc *)id;
+ struct s3c_rtc *info = id;
if (info->data->irq_handler)
info->data->irq_handler(info, S3C2410_INTP_ALM);
diff --git a/drivers/rtc/rtc-st-lpc.c b/drivers/rtc/rtc-st-lpc.c
index 82b0af1..75eb92c 100644
--- a/drivers/rtc/rtc-st-lpc.c
+++ b/drivers/rtc/rtc-st-lpc.c
@@ -74,7 +74,7 @@ static void st_rtc_set_hw_alarm(struct st_rtc *rtc,
static irqreturn_t st_rtc_handler(int this_irq, void *data)
{
- struct st_rtc *rtc = (struct st_rtc *)data;
+ struct st_rtc *rtc = data;
rtc_update_irq(rtc->rtc_dev, 1, RTC_AF);
diff --git a/drivers/rtc/rtc-stm32.c b/drivers/rtc/rtc-stm32.c
index 3a5c3d7..66fc7ba 100644
--- a/drivers/rtc/rtc-stm32.c
+++ b/drivers/rtc/rtc-stm32.c
@@ -169,7 +169,7 @@ static int stm32_rtc_wait_sync(struct stm32_rtc *rtc)
static irqreturn_t stm32_rtc_alarm_irq(int irq, void *dev_id)
{
- struct stm32_rtc *rtc = (struct stm32_rtc *)dev_id;
+ struct stm32_rtc *rtc = dev_id;
unsigned int isr, cr;
mutex_lock(&rtc->rtc_dev->ops_lock);
diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c
index 305c4d0..496a8dd 100644
--- a/drivers/rtc/rtc-sun6i.c
+++ b/drivers/rtc/rtc-sun6i.c
@@ -256,7 +256,7 @@ CLK_OF_DECLARE_DRIVER(sun6i_rtc_clk, "allwinner,sun6i-a31-rtc",
static irqreturn_t sun6i_rtc_alarmirq(int irq, void *id)
{
- struct sun6i_rtc_dev *chip = (struct sun6i_rtc_dev *) id;
+ struct sun6i_rtc_dev *chip = id;
irqreturn_t ret = IRQ_NONE;
u32 val;
diff --git a/drivers/rtc/rtc-sunxi.c b/drivers/rtc/rtc-sunxi.c
index abada60..f1f7fd7 100644
--- a/drivers/rtc/rtc-sunxi.c
+++ b/drivers/rtc/rtc-sunxi.c
@@ -158,7 +158,7 @@ struct sunxi_rtc_dev {
static irqreturn_t sunxi_rtc_alarmirq(int irq, void *id)
{
- struct sunxi_rtc_dev *chip = (struct sunxi_rtc_dev *) id;
+ struct sunxi_rtc_dev *chip = id;
u32 val;
val = readl(chip->base + SUNXI_ALRM_IRQ_STA);
diff --git a/drivers/rtc/rtc-vr41xx.c b/drivers/rtc/rtc-vr41xx.c
index 7ce2296..a0aa377 100644
--- a/drivers/rtc/rtc-vr41xx.c
+++ b/drivers/rtc/rtc-vr41xx.c
@@ -228,7 +228,7 @@ static int vr41xx_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
static irqreturn_t elapsedtime_interrupt(int irq, void *dev_id)
{
- struct platform_device *pdev = (struct platform_device *)dev_id;
+ struct platform_device *pdev = dev_id;
struct rtc_device *rtc = platform_get_drvdata(pdev);
rtc2_write(RTCINTREG, ELAPSEDTIME_INT);
@@ -240,7 +240,7 @@ static irqreturn_t elapsedtime_interrupt(int irq, void *dev_id)
static irqreturn_t rtclong1_interrupt(int irq, void *dev_id)
{
- struct platform_device *pdev = (struct platform_device *)dev_id;
+ struct platform_device *pdev = dev_id;
struct rtc_device *rtc = platform_get_drvdata(pdev);
unsigned long count = periodic_count;
diff --git a/drivers/rtc/rtc-xgene.c b/drivers/rtc/rtc-xgene.c
index 65b432a..2b8247d 100644
--- a/drivers/rtc/rtc-xgene.c
+++ b/drivers/rtc/rtc-xgene.c
@@ -131,7 +131,7 @@ static const struct rtc_class_ops xgene_rtc_ops = {
static irqreturn_t xgene_rtc_interrupt(int irq, void *id)
{
- struct xgene_rtc_dev *pdata = (struct xgene_rtc_dev *) id;
+ struct xgene_rtc_dev *pdata = id;
/* Check if interrupt asserted */
if (!(readl(pdata->csr_base + RTC_STAT) & RTC_STAT_BIT))
diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c
index da18a8a..f2f8a2d 100644
--- a/drivers/rtc/rtc-zynqmp.c
+++ b/drivers/rtc/rtc-zynqmp.c
@@ -193,7 +193,7 @@ static const struct rtc_class_ops xlnx_rtc_ops = {
static irqreturn_t xlnx_rtc_interrupt(int irq, void *id)
{
- struct xlnx_rtc_dev *xrtcdev = (struct xlnx_rtc_dev *)id;
+ struct xlnx_rtc_dev *xrtcdev = id;
unsigned int status;
status = readl(xrtcdev->reg_base + RTC_INT_STS);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] rtc: Use PTR_ERR_OR_ZERO
From: Alexandre Belloni @ 2017-08-29 14:20 UTC (permalink / raw)
To: Himanshu Jha; +Cc: a.zummo, baruch, linux-rtc, linux-arm-kernel, linux-kernel
In-Reply-To: <1504014419-16894-1-git-send-email-himanshujha199640@gmail.com>
Hi,
On 29/08/2017 at 19:16:59 +0530, Himanshu Jha wrote:
> Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR
>
I'm not taking that kind of useless changes (especially since it make
the code less readable) unless you have other significant improvement in
those drivers.
> Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
> ---
> drivers/rtc/rtc-digicolor.c | 5 +----
> drivers/rtc/rtc-ds1347.c | 5 +----
> 2 files changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/rtc/rtc-digicolor.c b/drivers/rtc/rtc-digicolor.c
> index b253bf1..1e200a0 100644
> --- a/drivers/rtc/rtc-digicolor.c
> +++ b/drivers/rtc/rtc-digicolor.c
> @@ -202,10 +202,7 @@ static int __init dc_rtc_probe(struct platform_device *pdev)
> platform_set_drvdata(pdev, rtc);
> rtc->rtc_dev = devm_rtc_device_register(&pdev->dev, pdev->name,
> &dc_rtc_ops, THIS_MODULE);
> - if (IS_ERR(rtc->rtc_dev))
> - return PTR_ERR(rtc->rtc_dev);
> -
> - return 0;
> + return PTR_ERR_OR_ZERO(rtc->rtc_dev);
> }
>
> static const struct of_device_id dc_dt_ids[] = {
> diff --git a/drivers/rtc/rtc-ds1347.c b/drivers/rtc/rtc-ds1347.c
> index ccfc9d4..9a02ca7 100644
> --- a/drivers/rtc/rtc-ds1347.c
> +++ b/drivers/rtc/rtc-ds1347.c
> @@ -155,10 +155,7 @@ static int ds1347_probe(struct spi_device *spi)
> rtc = devm_rtc_device_register(&spi->dev, "ds1347",
> &ds1347_rtc_ops, THIS_MODULE);
>
> - if (IS_ERR(rtc))
> - return PTR_ERR(rtc);
> -
> - return 0;
> + return PTR_ERR_OR_ZERO(rtc);
> }
>
> static struct spi_driver ds1347_driver = {
> --
> 2.7.4
>
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH] rtc: Use PTR_ERR_OR_ZERO
From: Himanshu Jha @ 2017-08-29 13:46 UTC (permalink / raw)
To: a.zummo
Cc: alexandre.belloni, baruch, linux-rtc, linux-arm-kernel,
linux-kernel, Himanshu Jha
Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR
Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
---
drivers/rtc/rtc-digicolor.c | 5 +----
drivers/rtc/rtc-ds1347.c | 5 +----
2 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/rtc/rtc-digicolor.c b/drivers/rtc/rtc-digicolor.c
index b253bf1..1e200a0 100644
--- a/drivers/rtc/rtc-digicolor.c
+++ b/drivers/rtc/rtc-digicolor.c
@@ -202,10 +202,7 @@ static int __init dc_rtc_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, rtc);
rtc->rtc_dev = devm_rtc_device_register(&pdev->dev, pdev->name,
&dc_rtc_ops, THIS_MODULE);
- if (IS_ERR(rtc->rtc_dev))
- return PTR_ERR(rtc->rtc_dev);
-
- return 0;
+ return PTR_ERR_OR_ZERO(rtc->rtc_dev);
}
static const struct of_device_id dc_dt_ids[] = {
diff --git a/drivers/rtc/rtc-ds1347.c b/drivers/rtc/rtc-ds1347.c
index ccfc9d4..9a02ca7 100644
--- a/drivers/rtc/rtc-ds1347.c
+++ b/drivers/rtc/rtc-ds1347.c
@@ -155,10 +155,7 @@ static int ds1347_probe(struct spi_device *spi)
rtc = devm_rtc_device_register(&spi->dev, "ds1347",
&ds1347_rtc_ops, THIS_MODULE);
- if (IS_ERR(rtc))
- return PTR_ERR(rtc);
-
- return 0;
+ return PTR_ERR_OR_ZERO(rtc);
}
static struct spi_driver ds1347_driver = {
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v3 4/5] input: Add MediaTek PMIC keys support
From: Chen Zhong @ 2017-08-29 7:40 UTC (permalink / raw)
To: Andi Shyti
Cc: Dmitry Torokhov, Rob Herring, Mark Rutland, Matthias Brugger,
Lee Jones, Eddie Huang, Alessandro Zummo, Alexandre Belloni,
Javier Martinez Canillas, Linus Walleij, Jaechul Lee, linux-input,
devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
linux-rtc
In-Reply-To: <20170829073453.GB4469@gangnam>
Hi Andi,
Sorry for that, i deleted some prefix since the namings will be such
long. If this should be consistent, I'll check and modify them in the
next version.
Thank you.
On Tue, 2017-08-29 at 16:34 +0900, Andi Shyti wrote:
> Hi Chen,
>
> what I would ask you is to be consistent with the naming, if your
> driver is called mtk-pmic, then all your functions and global
> variables should be named accordingly as mtk_pmic or MTK_PMIC.
> Please check in between the lines for real examples:
>
> > +#define PWRKEY_RST_EN_MASK 0x1
> > +#define PWRKEY_RST_EN_SHIFT 6
> > +#define HOMEKEY_RST_EN_MASK 0x1
> > +#define HOMEKEY_RST_EN_SHIFT 5
> > +#define RST_DU_MASK 0x3
> > +#define RST_DU_SHIFT 8
> > +
> > +#define PMIC_PWRKEY_INDEX 0
> > +#define PMIC_HOMEKEY_INDEX 1
> > +#define PMIC_MAX_KEY_COUNT 2
>
> #define MTK_PMIC_*
>
> > +struct pmic_keys_regs {
>
> struct mtk_pmic_keys_regs
>
> > + u32 deb_reg;
> > + u32 deb_mask;
> > + u32 intsel_reg;
> > + u32 intsel_mask;
> > +};
> > +
> > +#define PMIC_KEYS_REGS(_deb_reg, _deb_mask, _intsel_reg, _intsel_mask) \
>
> MTK_PMIC_KEYS_REGS
>
> > +{ \
> > + .deb_reg = _deb_reg, \
> > + .deb_mask = _deb_mask, \
> > + .intsel_reg = _intsel_reg, \
> > + .intsel_mask = _intsel_mask, \
> > +}
> > +
> > +struct pmic_regs {
>
> mtk_pmic_regs
>
> and so on... you got the point.
>
> Andi
^ permalink raw reply
* Re: [PATCH v3 4/5] input: Add MediaTek PMIC keys support
From: Andi Shyti @ 2017-08-29 7:34 UTC (permalink / raw)
To: Chen Zhong
Cc: Dmitry Torokhov, Rob Herring, Mark Rutland, Matthias Brugger,
Lee Jones, Eddie Huang, Alessandro Zummo, Alexandre Belloni,
Javier Martinez Canillas, Linus Walleij, Jaechul Lee, linux-input,
devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
linux-rtc
In-Reply-To: <1503642753-12385-5-git-send-email-chen.zhong@mediatek.com>
Hi Chen,
what I would ask you is to be consistent with the naming, if your
driver is called mtk-pmic, then all your functions and global
variables should be named accordingly as mtk_pmic or MTK_PMIC.
Please check in between the lines for real examples:
> +#define PWRKEY_RST_EN_MASK 0x1
> +#define PWRKEY_RST_EN_SHIFT 6
> +#define HOMEKEY_RST_EN_MASK 0x1
> +#define HOMEKEY_RST_EN_SHIFT 5
> +#define RST_DU_MASK 0x3
> +#define RST_DU_SHIFT 8
> +
> +#define PMIC_PWRKEY_INDEX 0
> +#define PMIC_HOMEKEY_INDEX 1
> +#define PMIC_MAX_KEY_COUNT 2
#define MTK_PMIC_*
> +struct pmic_keys_regs {
struct mtk_pmic_keys_regs
> + u32 deb_reg;
> + u32 deb_mask;
> + u32 intsel_reg;
> + u32 intsel_mask;
> +};
> +
> +#define PMIC_KEYS_REGS(_deb_reg, _deb_mask, _intsel_reg, _intsel_mask) \
MTK_PMIC_KEYS_REGS
> +{ \
> + .deb_reg = _deb_reg, \
> + .deb_mask = _deb_mask, \
> + .intsel_reg = _intsel_reg, \
> + .intsel_mask = _intsel_mask, \
> +}
> +
> +struct pmic_regs {
mtk_pmic_regs
and so on... you got the point.
Andi
^ permalink raw reply
* Re: [PATCH v3 4/5] input: Add MediaTek PMIC keys support
From: Chen Zhong @ 2017-08-29 2:11 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rob Herring, Mark Rutland, Matthias Brugger, Lee Jones,
Eddie Huang, Alessandro Zummo, Alexandre Belloni, Andi Shyti,
Javier Martinez Canillas, Linus Walleij, Jaechul Lee, linux-input,
devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
linux-rtc
In-Reply-To: <20170828165757.GE12195@dtor-ws>
On Mon, 2017-08-28 at 09:57 -0700, Dmitry Torokhov wrote:
> Hi Chen,
>
> On Fri, Aug 25, 2017 at 02:32:32PM +0800, Chen Zhong wrote:
> > +static int mtk_pmic_key_setup(struct mtk_pmic_keys *keys,
> > + struct pmic_keys_info *info)
> > +{
> > + int ret;
> > +
> > + info->keys = keys;
> > +
> > + ret = regmap_update_bits(keys->regmap, info->regs->intsel_reg,
> > + info->regs->intsel_mask,
> > + info->regs->intsel_mask);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = devm_request_threaded_irq(keys->dev, info->irq, NULL,
> > + mtk_pmic_keys_irq_handler_thread,
> > + IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
> > + "mtk-pmic-keys", info);
> > + if (ret) {
> > + dev_err(keys->dev, "Failed to request IRQ: %d: %d\n",
> > + info->irq, ret);
> > + return ret;
> > + }
> > +
> > + if (info->wakeup)
> > + irq_set_irq_wake(info->irq, 1);
>
> Normally we do this in suspend() (and undo in resume()), and I believe
> the drover API is enable_irq_wake() and disable_irq_wake().
>
Hi Dmitry,
I'll add suspend/resume callback functions and do this with
enable_irq_wake() and disable_irq_wake().
Thank you.
> Thanks.
>
^ permalink raw reply
* Re: [PATCH] rtc: ds1307: improve weekday handling
From: Alexandre Belloni @ 2017-08-28 20:25 UTC (permalink / raw)
To: Heiner Kallweit; +Cc: linux-rtc
In-Reply-To: <dea642c9-872e-1871-bdcd-13944d77d5be@gmail.com>
On 28/08/2017 at 21:58:23 +0200, Heiner Kallweit wrote:
> Am 28.08.2017 um 21:15 schrieb Alexandre Belloni:
> > On 28/08/2017 at 20:37:21 +0200, Heiner Kallweit wrote:
> >> mcp794xx as one chip supported by this driver needs the weekday for
> >> alarm matching. RTC core ignores the weekday so we can't rely on
> >> the values we receive in member tm_wday of struct rtc_time.
> >> Therefore calculate the weekday from date/time when setting the
> >> time and setting the alarm time for mcp794xx.
> >>
> >> When having this in place we don't have to check the weekday
> >> at each driver load.
> >> After a chip reset date/time and weekday may be out of sync but
> >> in this case date/time need to be set anyway.
> >>
> >
> > Nope, the core issue is that you can actually set an alarm in the future
> > without setting the time beforehand so this as to be fixed in the probe
> > (this was the issue as reported at the time of the fix).
> >
> Ah, found the original thread where this was discussed.
> Still I don't really understand the problem. mcp794xx matches based on
> secs, min, hour, wday, mday, month. When I use "rtcwake -s 5" I expect
> the alarm to trigger 5 secs from now. How can this ever work if the
> RTC has random date/time (wday being valid or not)?
>
wakealarm_store() reads the rtc time, adds the relative offset and sets
the alarm.
> I'd tend to say that if we know the RTC time is incorrect we shouldn't
> allow setting an alarm.
Unfortunately, this would break some platforms. Note that this is
already handled that way by some RTC drivers because they return an
error when it is known that the time is invalid.
I've started to work on that but it is far from finished.
> At least I wouldn't dare to program a bomb explosion time if I stand in
> front of it and know that the bomb's clock has a random value ;)
>
A timer will work as long as the clock is ticking, it doesn't matter
when it is started ;)
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH] rtc: ds1307: improve weekday handling
From: Heiner Kallweit @ 2017-08-28 20:12 UTC (permalink / raw)
To: Alexandre Belloni; +Cc: linux-rtc
In-Reply-To: <dea642c9-872e-1871-bdcd-13944d77d5be@gmail.com>
Am 28.08.2017 um 21:58 schrieb Heiner Kallweit:
> Am 28.08.2017 um 21:15 schrieb Alexandre Belloni:
>> On 28/08/2017 at 20:37:21 +0200, Heiner Kallweit wrote:
>>> mcp794xx as one chip supported by this driver needs the weekday for
>>> alarm matching. RTC core ignores the weekday so we can't rely on
>>> the values we receive in member tm_wday of struct rtc_time.
>>> Therefore calculate the weekday from date/time when setting the
>>> time and setting the alarm time for mcp794xx.
>>>
>>> When having this in place we don't have to check the weekday
>>> at each driver load.
>>> After a chip reset date/time and weekday may be out of sync but
>>> in this case date/time need to be set anyway.
>>>
>>
>> Nope, the core issue is that you can actually set an alarm in the future
>> without setting the time beforehand so this as to be fixed in the probe
>> (this was the issue as reported at the time of the fix).
>>
> Ah, found the original thread where this was discussed.
> Still I don't really understand the problem. mcp794xx matches based on
> secs, min, hour, wday, mday, month. When I use "rtcwake -s 5" I expect
> the alarm to trigger 5 secs from now. How can this ever work if the
> RTC has random date/time (wday being valid or not)?
>
OK, if "rtcwake -s 5" sets the alarm relative to the rtc time, not the
sys time then it would work. But allowing to use the rtc when it has
a random date / time still seems to be a weird use case to me.
> I'd tend to say that if we know the RTC time is incorrect we shouldn't
> allow setting an alarm.
> At least I wouldn't dare to program a bomb explosion time if I stand in
> front of it and know that the bomb's clock has a random value ;)
>
>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>> ---
>>> drivers/rtc/rtc-ds1307.c | 39 +++++++++++++++------------------------
>>> 1 file changed, 15 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
>>> index 9d680d36..83b8c997 100644
>>> --- a/drivers/rtc/rtc-ds1307.c
>>> +++ b/drivers/rtc/rtc-ds1307.c
>>> @@ -437,6 +437,18 @@ static int ds1307_get_time(struct device *dev, struct rtc_time *t)
>>> return rtc_valid_tm(t);
>>> }
>>>
>>> +/*
>>> + * Certain chips need the weekday for alarm matching and tm->t_wday
>>> + * may be not or not properly populated
>>> + */
>>> +static int ds1307_get_weekday(struct rtc_time *tm)
>>> +{
>>> + time64_t secs64 = rtc_tm_to_time64(tm);
>>> + int days = div_s64(secs64, 24 * 60 * 60);
>>> +
>>> + return (days + 4) % 7 + 1;
>>> +}
>>> +
>>> static int ds1307_set_time(struct device *dev, struct rtc_time *t)
>>> {
>>> struct ds1307 *ds1307 = dev_get_drvdata(dev);
>>> @@ -465,7 +477,7 @@ static int ds1307_set_time(struct device *dev, struct rtc_time *t)
>>> regs[DS1307_REG_SECS] = bin2bcd(t->tm_sec);
>>> regs[DS1307_REG_MIN] = bin2bcd(t->tm_min);
>>> regs[DS1307_REG_HOUR] = bin2bcd(t->tm_hour);
>>> - regs[DS1307_REG_WDAY] = bin2bcd(t->tm_wday + 1);
>>> + regs[DS1307_REG_WDAY] = ds1307_get_weekday(t);
>>> regs[DS1307_REG_MDAY] = bin2bcd(t->tm_mday);
>>> regs[DS1307_REG_MONTH] = bin2bcd(t->tm_mon + 1);
>>>
>>> @@ -902,7 +914,7 @@ static int mcp794xx_set_alarm(struct device *dev, struct rtc_wkalrm *t)
>>> regs[3] = bin2bcd(t->time.tm_sec);
>>> regs[4] = bin2bcd(t->time.tm_min);
>>> regs[5] = bin2bcd(t->time.tm_hour);
>>> - regs[6] = bin2bcd(t->time.tm_wday + 1);
>>> + regs[6] = ds1307_get_weekday(&t->time);
>>> regs[7] = bin2bcd(t->time.tm_mday);
>>> regs[8] = bin2bcd(t->time.tm_mon + 1);
>>>
>>> @@ -1355,14 +1367,12 @@ static int ds1307_probe(struct i2c_client *client,
>>> {
>>> struct ds1307 *ds1307;
>>> int err = -ENODEV;
>>> - int tmp, wday;
>>> + int tmp;
>>> const struct chip_desc *chip;
>>> bool want_irq;
>>> bool ds1307_can_wakeup_device = false;
>>> unsigned char regs[8];
>>> struct ds1307_platform_data *pdata = dev_get_platdata(&client->dev);
>>> - struct rtc_time tm;
>>> - unsigned long timestamp;
>>> u8 trickle_charger_setup = 0;
>>>
>>> ds1307 = devm_kzalloc(&client->dev, sizeof(struct ds1307), GFP_KERNEL);
>>> @@ -1642,25 +1652,6 @@ static int ds1307_probe(struct i2c_client *client,
>>> bin2bcd(tmp));
>>> }
>>>
>>> - /*
>>> - * Some IPs have weekday reset value = 0x1 which might not correct
>>> - * hence compute the wday using the current date/month/year values
>>> - */
>>> - ds1307_get_time(ds1307->dev, &tm);
>>> - wday = tm.tm_wday;
>>> - timestamp = rtc_tm_to_time64(&tm);
>>> - rtc_time64_to_tm(timestamp, &tm);
>>> -
>>> - /*
>>> - * Check if reset wday is different from the computed wday
>>> - * If different then set the wday which we computed using
>>> - * timestamp
>>> - */
>>> - if (wday != tm.tm_wday)
>>> - regmap_update_bits(ds1307->regmap, MCP794XX_REG_WEEKDAY,
>>> - MCP794XX_REG_WEEKDAY_WDAY_MASK,
>>> - tm.tm_wday + 1);
>>> -
>>> if (want_irq || ds1307_can_wakeup_device) {
>>> device_set_wakeup_capable(ds1307->dev, true);
>>> set_bit(HAS_ALARM, &ds1307->flags);
>>> --
>>> 2.14.1
>>>
>>
>
^ permalink raw reply
* Re: [PATCH] rtc: ds1307: improve weekday handling
From: Heiner Kallweit @ 2017-08-28 19:58 UTC (permalink / raw)
To: Alexandre Belloni; +Cc: linux-rtc
In-Reply-To: <20170828191518.ofntjb2tknttjexq@piout.net>
Am 28.08.2017 um 21:15 schrieb Alexandre Belloni:
> On 28/08/2017 at 20:37:21 +0200, Heiner Kallweit wrote:
>> mcp794xx as one chip supported by this driver needs the weekday for
>> alarm matching. RTC core ignores the weekday so we can't rely on
>> the values we receive in member tm_wday of struct rtc_time.
>> Therefore calculate the weekday from date/time when setting the
>> time and setting the alarm time for mcp794xx.
>>
>> When having this in place we don't have to check the weekday
>> at each driver load.
>> After a chip reset date/time and weekday may be out of sync but
>> in this case date/time need to be set anyway.
>>
>
> Nope, the core issue is that you can actually set an alarm in the future
> without setting the time beforehand so this as to be fixed in the probe
> (this was the issue as reported at the time of the fix).
>
Ah, found the original thread where this was discussed.
Still I don't really understand the problem. mcp794xx matches based on
secs, min, hour, wday, mday, month. When I use "rtcwake -s 5" I expect
the alarm to trigger 5 secs from now. How can this ever work if the
RTC has random date/time (wday being valid or not)?
I'd tend to say that if we know the RTC time is incorrect we shouldn't
allow setting an alarm.
At least I wouldn't dare to program a bomb explosion time if I stand in
front of it and know that the bomb's clock has a random value ;)
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>> drivers/rtc/rtc-ds1307.c | 39 +++++++++++++++------------------------
>> 1 file changed, 15 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
>> index 9d680d36..83b8c997 100644
>> --- a/drivers/rtc/rtc-ds1307.c
>> +++ b/drivers/rtc/rtc-ds1307.c
>> @@ -437,6 +437,18 @@ static int ds1307_get_time(struct device *dev, struct rtc_time *t)
>> return rtc_valid_tm(t);
>> }
>>
>> +/*
>> + * Certain chips need the weekday for alarm matching and tm->t_wday
>> + * may be not or not properly populated
>> + */
>> +static int ds1307_get_weekday(struct rtc_time *tm)
>> +{
>> + time64_t secs64 = rtc_tm_to_time64(tm);
>> + int days = div_s64(secs64, 24 * 60 * 60);
>> +
>> + return (days + 4) % 7 + 1;
>> +}
>> +
>> static int ds1307_set_time(struct device *dev, struct rtc_time *t)
>> {
>> struct ds1307 *ds1307 = dev_get_drvdata(dev);
>> @@ -465,7 +477,7 @@ static int ds1307_set_time(struct device *dev, struct rtc_time *t)
>> regs[DS1307_REG_SECS] = bin2bcd(t->tm_sec);
>> regs[DS1307_REG_MIN] = bin2bcd(t->tm_min);
>> regs[DS1307_REG_HOUR] = bin2bcd(t->tm_hour);
>> - regs[DS1307_REG_WDAY] = bin2bcd(t->tm_wday + 1);
>> + regs[DS1307_REG_WDAY] = ds1307_get_weekday(t);
>> regs[DS1307_REG_MDAY] = bin2bcd(t->tm_mday);
>> regs[DS1307_REG_MONTH] = bin2bcd(t->tm_mon + 1);
>>
>> @@ -902,7 +914,7 @@ static int mcp794xx_set_alarm(struct device *dev, struct rtc_wkalrm *t)
>> regs[3] = bin2bcd(t->time.tm_sec);
>> regs[4] = bin2bcd(t->time.tm_min);
>> regs[5] = bin2bcd(t->time.tm_hour);
>> - regs[6] = bin2bcd(t->time.tm_wday + 1);
>> + regs[6] = ds1307_get_weekday(&t->time);
>> regs[7] = bin2bcd(t->time.tm_mday);
>> regs[8] = bin2bcd(t->time.tm_mon + 1);
>>
>> @@ -1355,14 +1367,12 @@ static int ds1307_probe(struct i2c_client *client,
>> {
>> struct ds1307 *ds1307;
>> int err = -ENODEV;
>> - int tmp, wday;
>> + int tmp;
>> const struct chip_desc *chip;
>> bool want_irq;
>> bool ds1307_can_wakeup_device = false;
>> unsigned char regs[8];
>> struct ds1307_platform_data *pdata = dev_get_platdata(&client->dev);
>> - struct rtc_time tm;
>> - unsigned long timestamp;
>> u8 trickle_charger_setup = 0;
>>
>> ds1307 = devm_kzalloc(&client->dev, sizeof(struct ds1307), GFP_KERNEL);
>> @@ -1642,25 +1652,6 @@ static int ds1307_probe(struct i2c_client *client,
>> bin2bcd(tmp));
>> }
>>
>> - /*
>> - * Some IPs have weekday reset value = 0x1 which might not correct
>> - * hence compute the wday using the current date/month/year values
>> - */
>> - ds1307_get_time(ds1307->dev, &tm);
>> - wday = tm.tm_wday;
>> - timestamp = rtc_tm_to_time64(&tm);
>> - rtc_time64_to_tm(timestamp, &tm);
>> -
>> - /*
>> - * Check if reset wday is different from the computed wday
>> - * If different then set the wday which we computed using
>> - * timestamp
>> - */
>> - if (wday != tm.tm_wday)
>> - regmap_update_bits(ds1307->regmap, MCP794XX_REG_WEEKDAY,
>> - MCP794XX_REG_WEEKDAY_WDAY_MASK,
>> - tm.tm_wday + 1);
>> -
>> if (want_irq || ds1307_can_wakeup_device) {
>> device_set_wakeup_capable(ds1307->dev, true);
>> set_bit(HAS_ALARM, &ds1307->flags);
>> --
>> 2.14.1
>>
>
^ permalink raw reply
* Re: [PATCH] rtc: ds1307: improve weekday handling
From: Alexandre Belloni @ 2017-08-28 19:15 UTC (permalink / raw)
To: Heiner Kallweit; +Cc: linux-rtc
In-Reply-To: <57598a86-21c7-b354-2840-309563440435@gmail.com>
On 28/08/2017 at 20:37:21 +0200, Heiner Kallweit wrote:
> mcp794xx as one chip supported by this driver needs the weekday for
> alarm matching. RTC core ignores the weekday so we can't rely on
> the values we receive in member tm_wday of struct rtc_time.
> Therefore calculate the weekday from date/time when setting the
> time and setting the alarm time for mcp794xx.
>
> When having this in place we don't have to check the weekday
> at each driver load.
> After a chip reset date/time and weekday may be out of sync but
> in this case date/time need to be set anyway.
>
Nope, the core issue is that you can actually set an alarm in the future
without setting the time beforehand so this as to be fixed in the probe
(this was the issue as reported at the time of the fix).
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> drivers/rtc/rtc-ds1307.c | 39 +++++++++++++++------------------------
> 1 file changed, 15 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
> index 9d680d36..83b8c997 100644
> --- a/drivers/rtc/rtc-ds1307.c
> +++ b/drivers/rtc/rtc-ds1307.c
> @@ -437,6 +437,18 @@ static int ds1307_get_time(struct device *dev, struct rtc_time *t)
> return rtc_valid_tm(t);
> }
>
> +/*
> + * Certain chips need the weekday for alarm matching and tm->t_wday
> + * may be not or not properly populated
> + */
> +static int ds1307_get_weekday(struct rtc_time *tm)
> +{
> + time64_t secs64 = rtc_tm_to_time64(tm);
> + int days = div_s64(secs64, 24 * 60 * 60);
> +
> + return (days + 4) % 7 + 1;
> +}
> +
> static int ds1307_set_time(struct device *dev, struct rtc_time *t)
> {
> struct ds1307 *ds1307 = dev_get_drvdata(dev);
> @@ -465,7 +477,7 @@ static int ds1307_set_time(struct device *dev, struct rtc_time *t)
> regs[DS1307_REG_SECS] = bin2bcd(t->tm_sec);
> regs[DS1307_REG_MIN] = bin2bcd(t->tm_min);
> regs[DS1307_REG_HOUR] = bin2bcd(t->tm_hour);
> - regs[DS1307_REG_WDAY] = bin2bcd(t->tm_wday + 1);
> + regs[DS1307_REG_WDAY] = ds1307_get_weekday(t);
> regs[DS1307_REG_MDAY] = bin2bcd(t->tm_mday);
> regs[DS1307_REG_MONTH] = bin2bcd(t->tm_mon + 1);
>
> @@ -902,7 +914,7 @@ static int mcp794xx_set_alarm(struct device *dev, struct rtc_wkalrm *t)
> regs[3] = bin2bcd(t->time.tm_sec);
> regs[4] = bin2bcd(t->time.tm_min);
> regs[5] = bin2bcd(t->time.tm_hour);
> - regs[6] = bin2bcd(t->time.tm_wday + 1);
> + regs[6] = ds1307_get_weekday(&t->time);
> regs[7] = bin2bcd(t->time.tm_mday);
> regs[8] = bin2bcd(t->time.tm_mon + 1);
>
> @@ -1355,14 +1367,12 @@ static int ds1307_probe(struct i2c_client *client,
> {
> struct ds1307 *ds1307;
> int err = -ENODEV;
> - int tmp, wday;
> + int tmp;
> const struct chip_desc *chip;
> bool want_irq;
> bool ds1307_can_wakeup_device = false;
> unsigned char regs[8];
> struct ds1307_platform_data *pdata = dev_get_platdata(&client->dev);
> - struct rtc_time tm;
> - unsigned long timestamp;
> u8 trickle_charger_setup = 0;
>
> ds1307 = devm_kzalloc(&client->dev, sizeof(struct ds1307), GFP_KERNEL);
> @@ -1642,25 +1652,6 @@ static int ds1307_probe(struct i2c_client *client,
> bin2bcd(tmp));
> }
>
> - /*
> - * Some IPs have weekday reset value = 0x1 which might not correct
> - * hence compute the wday using the current date/month/year values
> - */
> - ds1307_get_time(ds1307->dev, &tm);
> - wday = tm.tm_wday;
> - timestamp = rtc_tm_to_time64(&tm);
> - rtc_time64_to_tm(timestamp, &tm);
> -
> - /*
> - * Check if reset wday is different from the computed wday
> - * If different then set the wday which we computed using
> - * timestamp
> - */
> - if (wday != tm.tm_wday)
> - regmap_update_bits(ds1307->regmap, MCP794XX_REG_WEEKDAY,
> - MCP794XX_REG_WEEKDAY_WDAY_MASK,
> - tm.tm_wday + 1);
> -
> if (want_irq || ds1307_can_wakeup_device) {
> device_set_wakeup_capable(ds1307->dev, true);
> set_bit(HAS_ALARM, &ds1307->flags);
> --
> 2.14.1
>
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH] rtc: core: let rtc_set_time properly populate element wday
From: Alexandre Belloni @ 2017-08-28 18:54 UTC (permalink / raw)
To: Heiner Kallweit; +Cc: linux-rtc
In-Reply-To: <9a11b823-abe0-6611-9d97-75e2a2ec7745@gmail.com>
On 28/08/2017 at 20:13:25 +0200, Heiner Kallweit wrote:
> Am 28.08.2017 um 09:54 schrieb Alexandre Belloni:
> > On 27/08/2017 at 21:35:34 +0200, Heiner Kallweit wrote:
> >> When having a look at the MCP794XX datasheet it became clear why the
> >> mentioned weekday-related patch was submitted. This chip is quite
> >> strange regarding the possible alarm match conditions:
> >>
> >> ALMxMSK<2:0>: Alarm Mask bits
> >> 000 = Seconds match
> >> 001 = Minutes match
> >> 010 = Hours match (logic takes into account 12-/24-hour operation)
> >> 011 = Day of week match
> >> 100 = Date match
> >> 101 = Reserved; do not use
> >> 110 = Reserved; do not use
> >> 111 = Seconds, Minutes, Hour, Day of Week, Date and Month
> >>
> >> When not having a proper weekday you get only either seconds or
> >> minutes or hours or date match.
> >>
> >> So it's best to ensure that the weekday is properly populated.
> >> However we can do this in the driver and don't have to touch the core.
> >>
> >
> > Can you do that only for mcp794xx? I received one earlier this month and
> > I'll separate the driver from rtc-ds1307.c at some point.
> >
> Sure, I'll submit a patch for this. Separating the driver for mcp794xx may
> not be needed as it is not that different from the other chips supported
> by this driver. Once I have my patch series proposal towards a ds1307_lib
> in a little more clean state this should become clearer.
>
I would like to find a better name than that but I didn't come up with
anything yet... feel free to get creative ;)
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH] rtc: ds1307: improve weekday handling
From: Heiner Kallweit @ 2017-08-28 18:37 UTC (permalink / raw)
To: Alexandre Belloni; +Cc: linux-rtc
mcp794xx as one chip supported by this driver needs the weekday for
alarm matching. RTC core ignores the weekday so we can't rely on
the values we receive in member tm_wday of struct rtc_time.
Therefore calculate the weekday from date/time when setting the
time and setting the alarm time for mcp794xx.
When having this in place we don't have to check the weekday
at each driver load.
After a chip reset date/time and weekday may be out of sync but
in this case date/time need to be set anyway.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/rtc/rtc-ds1307.c | 39 +++++++++++++++------------------------
1 file changed, 15 insertions(+), 24 deletions(-)
diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 9d680d36..83b8c997 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -437,6 +437,18 @@ static int ds1307_get_time(struct device *dev, struct rtc_time *t)
return rtc_valid_tm(t);
}
+/*
+ * Certain chips need the weekday for alarm matching and tm->t_wday
+ * may be not or not properly populated
+ */
+static int ds1307_get_weekday(struct rtc_time *tm)
+{
+ time64_t secs64 = rtc_tm_to_time64(tm);
+ int days = div_s64(secs64, 24 * 60 * 60);
+
+ return (days + 4) % 7 + 1;
+}
+
static int ds1307_set_time(struct device *dev, struct rtc_time *t)
{
struct ds1307 *ds1307 = dev_get_drvdata(dev);
@@ -465,7 +477,7 @@ static int ds1307_set_time(struct device *dev, struct rtc_time *t)
regs[DS1307_REG_SECS] = bin2bcd(t->tm_sec);
regs[DS1307_REG_MIN] = bin2bcd(t->tm_min);
regs[DS1307_REG_HOUR] = bin2bcd(t->tm_hour);
- regs[DS1307_REG_WDAY] = bin2bcd(t->tm_wday + 1);
+ regs[DS1307_REG_WDAY] = ds1307_get_weekday(t);
regs[DS1307_REG_MDAY] = bin2bcd(t->tm_mday);
regs[DS1307_REG_MONTH] = bin2bcd(t->tm_mon + 1);
@@ -902,7 +914,7 @@ static int mcp794xx_set_alarm(struct device *dev, struct rtc_wkalrm *t)
regs[3] = bin2bcd(t->time.tm_sec);
regs[4] = bin2bcd(t->time.tm_min);
regs[5] = bin2bcd(t->time.tm_hour);
- regs[6] = bin2bcd(t->time.tm_wday + 1);
+ regs[6] = ds1307_get_weekday(&t->time);
regs[7] = bin2bcd(t->time.tm_mday);
regs[8] = bin2bcd(t->time.tm_mon + 1);
@@ -1355,14 +1367,12 @@ static int ds1307_probe(struct i2c_client *client,
{
struct ds1307 *ds1307;
int err = -ENODEV;
- int tmp, wday;
+ int tmp;
const struct chip_desc *chip;
bool want_irq;
bool ds1307_can_wakeup_device = false;
unsigned char regs[8];
struct ds1307_platform_data *pdata = dev_get_platdata(&client->dev);
- struct rtc_time tm;
- unsigned long timestamp;
u8 trickle_charger_setup = 0;
ds1307 = devm_kzalloc(&client->dev, sizeof(struct ds1307), GFP_KERNEL);
@@ -1642,25 +1652,6 @@ static int ds1307_probe(struct i2c_client *client,
bin2bcd(tmp));
}
- /*
- * Some IPs have weekday reset value = 0x1 which might not correct
- * hence compute the wday using the current date/month/year values
- */
- ds1307_get_time(ds1307->dev, &tm);
- wday = tm.tm_wday;
- timestamp = rtc_tm_to_time64(&tm);
- rtc_time64_to_tm(timestamp, &tm);
-
- /*
- * Check if reset wday is different from the computed wday
- * If different then set the wday which we computed using
- * timestamp
- */
- if (wday != tm.tm_wday)
- regmap_update_bits(ds1307->regmap, MCP794XX_REG_WEEKDAY,
- MCP794XX_REG_WEEKDAY_WDAY_MASK,
- tm.tm_wday + 1);
-
if (want_irq || ds1307_can_wakeup_device) {
device_set_wakeup_capable(ds1307->dev, true);
set_bit(HAS_ALARM, &ds1307->flags);
--
2.14.1
^ permalink raw reply related
* Re: [PATCH] rtc: core: let rtc_set_time properly populate element wday
From: Heiner Kallweit @ 2017-08-28 18:13 UTC (permalink / raw)
To: Alexandre Belloni; +Cc: linux-rtc
In-Reply-To: <20170828075458.4cdi7pet3yxeskqc@piout.net>
Am 28.08.2017 um 09:54 schrieb Alexandre Belloni:
> On 27/08/2017 at 21:35:34 +0200, Heiner Kallweit wrote:
>> When having a look at the MCP794XX datasheet it became clear why the
>> mentioned weekday-related patch was submitted. This chip is quite
>> strange regarding the possible alarm match conditions:
>>
>> ALMxMSK<2:0>: Alarm Mask bits
>> 000 = Seconds match
>> 001 = Minutes match
>> 010 = Hours match (logic takes into account 12-/24-hour operation)
>> 011 = Day of week match
>> 100 = Date match
>> 101 = Reserved; do not use
>> 110 = Reserved; do not use
>> 111 = Seconds, Minutes, Hour, Day of Week, Date and Month
>>
>> When not having a proper weekday you get only either seconds or
>> minutes or hours or date match.
>>
>> So it's best to ensure that the weekday is properly populated.
>> However we can do this in the driver and don't have to touch the core.
>>
>
> Can you do that only for mcp794xx? I received one earlier this month and
> I'll separate the driver from rtc-ds1307.c at some point.
>
Sure, I'll submit a patch for this. Separating the driver for mcp794xx may
not be needed as it is not that different from the other chips supported
by this driver. Once I have my patch series proposal towards a ds1307_lib
in a little more clean state this should become clearer.
Regards, Heiner
> BTW, I've rebased and pushed some cleanup for this driver in:
> https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git/log/?h=rtc-ds1307
>
> Can you base yourself on that?
>
OK
> Thanks,
>
^ permalink raw reply
* Re: [PATCH v3 4/5] input: Add MediaTek PMIC keys support
From: Dmitry Torokhov @ 2017-08-28 16:57 UTC (permalink / raw)
To: Chen Zhong
Cc: Rob Herring, Mark Rutland, Matthias Brugger, Lee Jones,
Eddie Huang, Alessandro Zummo, Alexandre Belloni, Andi Shyti,
Javier Martinez Canillas, Linus Walleij, Jaechul Lee, linux-input,
devicetree, linux-arm-kernel, linux-mediatek, linux-kernel,
linux-rtc
In-Reply-To: <1503642753-12385-5-git-send-email-chen.zhong@mediatek.com>
Hi Chen,
On Fri, Aug 25, 2017 at 02:32:32PM +0800, Chen Zhong wrote:
> +static int mtk_pmic_key_setup(struct mtk_pmic_keys *keys,
> + struct pmic_keys_info *info)
> +{
> + int ret;
> +
> + info->keys = keys;
> +
> + ret = regmap_update_bits(keys->regmap, info->regs->intsel_reg,
> + info->regs->intsel_mask,
> + info->regs->intsel_mask);
> + if (ret < 0)
> + return ret;
> +
> + ret = devm_request_threaded_irq(keys->dev, info->irq, NULL,
> + mtk_pmic_keys_irq_handler_thread,
> + IRQF_ONESHOT | IRQF_TRIGGER_HIGH,
> + "mtk-pmic-keys", info);
> + if (ret) {
> + dev_err(keys->dev, "Failed to request IRQ: %d: %d\n",
> + info->irq, ret);
> + return ret;
> + }
> +
> + if (info->wakeup)
> + irq_set_irq_wake(info->irq, 1);
Normally we do this in suspend() (and undo in resume()), and I believe
the drover API is enable_irq_wake() and disable_irq_wake().
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3 2/3] rtc: Add Realtek RTD1295
From: Alexandre Belloni @ 2017-08-28 16:16 UTC (permalink / raw)
To: Andreas Färber
Cc: Alessandro Zummo, linux-rtc, linux-arm-kernel, linux-kernel,
Roc He, 蒋丽琴
In-Reply-To: <20170828111041.17946-3-afaerber@suse.de>
Hi,
Seems fine to me, one small comment:
On 28/08/2017 at 13:10:40 +0200, Andreas Färber wrote:
> +static int rtd119x_rtc_remove(struct platform_device *pdev)
> +{
> + struct rtd119x_rtc *data = platform_get_drvdata(pdev);
> +
> + rtd119x_rtc_set_enabled(&pdev->dev, false);
> +
> + clk_disable_unprepare(data->clk);
Missing clk_put ?
> +
> + return 0;
> +}
> +
> +static struct platform_driver rtd119x_rtc_driver = {
> + .probe = rtd119x_rtc_probe,
> + .remove = rtd119x_rtc_remove,
> + .driver = {
> + .name = "rtd1295-rtc",
> + .of_match_table = rtd119x_rtc_dt_ids,
> + },
> +};
> +builtin_platform_driver(rtd119x_rtc_driver);
> --
> 2.12.3
>
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v2 2/3] rtc: Add Realtek RTD1295
From: Alexandre Belloni @ 2017-08-28 15:50 UTC (permalink / raw)
To: Andreas Färber
Cc: Alessandro Zummo, linux-rtc, linux-arm-kernel, linux-kernel,
Roc He, 蒋丽琴, Andrew Lunn
In-Reply-To: <d20fbc21-bd8f-c191-ed51-03487868e7b0@suse.de>
Hi,
On 27/08/2017 at 13:30:59 +0200, Andreas Färber wrote:
> Well, I found your rtc_year_days rather confusing and had to play with
> the arguments until I got it working as expected, so I wanted an inline
> function (or macro) as abstraction from my three callers.
>
> Sadly the naming is rather confusing as I am looking for the number of
> days 365..366, whereas your rtc_year_days is meant to return 0..365 and
> I would just like to extract the 12th array element but need to counter
> the -1 subtraction. rtc_year_days(31, 11, year) + 1 is not intuitive
> either - reads like November (and ranges are not documented).
>
> What about exporting a convenient rtc_days_in_year(year) from rtc-lib.c
> accessing the table directly without rtc_year_days detour? Alternatively
> an inline function in rtc.h to the same effect without the array?
>
This could have been done but what you did in your v3 is fine too. It
will always be time to move that to the core later.
> Also despite is_leap_year() returning a bool || expression you keep
> using it as array index or integer to add. That assumes true == 1,
> whereas to my knowledge only false is guaranteed to be 0 and any
> non-zero value means true. So I'd expect the code to be like this:
sizeof(bool) (actually _Bool) is 1 so it can only be 0 or 1.
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH v3 3/3] arm64: dts: realtek: Add RTD1295 RTC node
From: Andreas Färber @ 2017-08-28 11:10 UTC (permalink / raw)
To: Alessandro Zummo, Alexandre Belloni, linux-rtc, linux-arm-kernel
Cc: linux-kernel, Roc He, 蒋丽琴,
Andreas Färber, Rob Herring, Mark Rutland, Catalin Marinas,
Will Deacon, devicetree
In-Reply-To: <20170828111041.17946-1-afaerber@suse.de>
Add RTC node to the Realtek RTD1295 Device Tree.
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
v2 -> v3:
* Rebased to no longer depend on reset controller series
v1 -> v2: Unchanged
Depends on the pending clock bindings.
arch/arm64/boot/dts/realtek/rtd1295.dtsi | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm64/boot/dts/realtek/rtd1295.dtsi b/arch/arm64/boot/dts/realtek/rtd1295.dtsi
index ab72255f843f..fbbd6456079e 100644
--- a/arch/arm64/boot/dts/realtek/rtd1295.dtsi
+++ b/arch/arm64/boot/dts/realtek/rtd1295.dtsi
@@ -164,6 +164,12 @@
status = "disabled";
};
+ rtc@9801b600 {
+ compatible = "realtek,rtd1295-rtc";
+ reg = <0x9801b600 0x100>;
+ clocks = <&clkc RTD1295_CLK_EN_MISC_RTC>;
+ };
+
gic: interrupt-controller@ff011000 {
compatible = "arm,gic-400";
reg = <0xff011000 0x1000>,
--
2.12.3
^ permalink raw reply related
* [PATCH v3 2/3] rtc: Add Realtek RTD1295
From: Andreas Färber @ 2017-08-28 11:10 UTC (permalink / raw)
To: Alessandro Zummo, Alexandre Belloni, linux-rtc, linux-arm-kernel
Cc: linux-kernel, Roc He, 蒋丽琴,
Andreas Färber
In-Reply-To: <20170828111041.17946-1-afaerber@suse.de>
Based on QNAP's arch/arm/mach-rtk119x/driver/rtk_rtc_drv.c code and
mach-rtk119x/driver/dc2vo/fpga/include/mis_reg.h register definitions.
The base year 2014 was observed on all of Zidoo X9S, ProBox2 Ava and
Beelink Lake I.
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
v2 -> v3:
* Dropped spinlock (Andrew)
* Refactored days-in-year helper to avoid rtc_year_days()
v1 -> v2:
* Dropped open/release in favor of probe/remove (Alexandre)
* read_time: Reordered register accesses (Alexandre)
* read_time/set_time: Refactored day calculations to avoid time64_t (Alexandre)
* read_time: Retry if seconds change (Alexandre)
* probe: Added missing RTCACR initialization code
* set_time: Fixed year check (off by 1900)
* set_time: Fixed new seconds (off by factor two)
* Cleaned up debug output (Andrew)
* Added spinlocks around register accesses
* Added masks for register fields
drivers/rtc/Kconfig | 8 ++
drivers/rtc/Makefile | 1 +
drivers/rtc/rtc-rtd119x.c | 241 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 250 insertions(+)
create mode 100644 drivers/rtc/rtc-rtd119x.c
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 22efa21b1d81..d5a46f311ecb 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1765,6 +1765,14 @@ config RTC_DRV_CPCAP
Say y here for CPCAP rtc found on some Motorola phones
and tablets such as Droid 4.
+config RTC_DRV_RTD119X
+ bool "Realtek RTD129x RTC"
+ depends on ARCH_REALTEK || COMPILE_TEST
+ default ARCH_REALTEK
+ help
+ If you say yes here, you get support for the RTD1295 SoC
+ Real Time Clock.
+
comment "HID Sensor RTC drivers"
config RTC_DRV_HID_SENSOR_TIME
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index acd366b41c85..55a0a5ca45b0 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -131,6 +131,7 @@ obj-$(CONFIG_RTC_DRV_RP5C01) += rtc-rp5c01.o
obj-$(CONFIG_RTC_DRV_RS5C313) += rtc-rs5c313.o
obj-$(CONFIG_RTC_DRV_RS5C348) += rtc-rs5c348.o
obj-$(CONFIG_RTC_DRV_RS5C372) += rtc-rs5c372.o
+obj-$(CONFIG_RTC_DRV_RTD119X) += rtc-rtd119x.o
obj-$(CONFIG_RTC_DRV_RV3029C2) += rtc-rv3029c2.o
obj-$(CONFIG_RTC_DRV_RV8803) += rtc-rv8803.o
obj-$(CONFIG_RTC_DRV_RX4581) += rtc-rx4581.o
diff --git a/drivers/rtc/rtc-rtd119x.c b/drivers/rtc/rtc-rtd119x.c
new file mode 100644
index 000000000000..526f586d0355
--- /dev/null
+++ b/drivers/rtc/rtc-rtd119x.c
@@ -0,0 +1,241 @@
+/*
+ * Realtek RTD129x RTC
+ *
+ * Copyright (c) 2017 Andreas Färber
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/rtc.h>
+#include <linux/spinlock.h>
+
+#define RTD_RTCSEC 0x00
+#define RTD_RTCMIN 0x04
+#define RTD_RTCHR 0x08
+#define RTD_RTCDATE1 0x0c
+#define RTD_RTCDATE2 0x10
+#define RTD_RTCACR 0x28
+#define RTD_RTCEN 0x2c
+#define RTD_RTCCR 0x30
+
+#define RTD_RTCSEC_RTCSEC_MASK 0x7f
+
+#define RTD_RTCMIN_RTCMIN_MASK 0x3f
+
+#define RTD_RTCHR_RTCHR_MASK 0x1f
+
+#define RTD_RTCDATE1_RTCDATE1_MASK 0xff
+
+#define RTD_RTCDATE2_RTCDATE2_MASK 0x7f
+
+#define RTD_RTCACR_RTCPWR BIT(7)
+
+#define RTD_RTCEN_RTCEN_MASK 0xff
+
+#define RTD_RTCCR_RTCRST BIT(6)
+
+struct rtd119x_rtc {
+ void __iomem *base;
+ struct clk *clk;
+ struct rtc_device *rtcdev;
+ unsigned int base_year;
+};
+
+static inline int rtd119x_rtc_days_in_year(int year)
+{
+ return 365 + (is_leap_year(year) ? 1 : 0);
+}
+
+static void rtd119x_rtc_reset(struct device *dev)
+{
+ struct rtd119x_rtc *data = dev_get_drvdata(dev);
+ u32 val;
+
+ val = readl_relaxed(data->base + RTD_RTCCR);
+ val |= RTD_RTCCR_RTCRST;
+ writel_relaxed(val, data->base + RTD_RTCCR);
+
+ val &= ~RTD_RTCCR_RTCRST;
+ writel(val, data->base + RTD_RTCCR);
+}
+
+static void rtd119x_rtc_set_enabled(struct device *dev, bool enable)
+{
+ struct rtd119x_rtc *data = dev_get_drvdata(dev);
+ u32 val;
+
+ val = readl_relaxed(data->base + RTD_RTCEN);
+ if (enable) {
+ if ((val & RTD_RTCEN_RTCEN_MASK) == 0x5a)
+ return;
+ writel_relaxed(0x5a, data->base + RTD_RTCEN);
+ } else {
+ writel_relaxed(0, data->base + RTD_RTCEN);
+ }
+}
+
+static int rtd119x_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+ struct rtd119x_rtc *data = dev_get_drvdata(dev);
+ s32 day;
+ u32 sec;
+ unsigned int year;
+ int tries = 0;
+
+ while (true) {
+ tm->tm_sec = (readl_relaxed(data->base + RTD_RTCSEC) & RTD_RTCSEC_RTCSEC_MASK) >> 1;
+ tm->tm_min = readl_relaxed(data->base + RTD_RTCMIN) & RTD_RTCMIN_RTCMIN_MASK;
+ tm->tm_hour = readl_relaxed(data->base + RTD_RTCHR) & RTD_RTCHR_RTCHR_MASK;
+ day = readl_relaxed(data->base + RTD_RTCDATE1) & RTD_RTCDATE1_RTCDATE1_MASK;
+ day |= (readl_relaxed(data->base + RTD_RTCDATE2) & RTD_RTCDATE2_RTCDATE2_MASK) << 8;
+ sec = (readl_relaxed(data->base + RTD_RTCSEC) & RTD_RTCSEC_RTCSEC_MASK) >> 1;
+ tries++;
+
+ if (sec == tm->tm_sec)
+ break;
+
+ if (tries >= 3)
+ return -EINVAL;
+ }
+ if (tries > 1)
+ dev_dbg(dev, "%s: needed %i tries\n", __func__, tries);
+
+ year = data->base_year;
+ while (day >= rtd119x_rtc_days_in_year(year)) {
+ day -= rtd119x_rtc_days_in_year(year);
+ year++;
+ }
+ tm->tm_year = year - 1900;
+ tm->tm_yday = day;
+
+ tm->tm_mon = 0;
+ while (day >= rtc_month_days(tm->tm_mon, year)) {
+ day -= rtc_month_days(tm->tm_mon, year);
+ tm->tm_mon++;
+ }
+ tm->tm_mday = day + 1;
+
+ return 0;
+}
+
+static int rtd119x_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+ struct rtd119x_rtc *data = dev_get_drvdata(dev);
+ unsigned int day;
+ int i;
+
+ if (1900 + tm->tm_year < data->base_year)
+ return -EINVAL;
+
+ day = 0;
+ for (i = data->base_year; i < 1900 + tm->tm_year; i++)
+ day += rtd119x_rtc_days_in_year(i);
+
+ day += tm->tm_yday;
+ if (day > 0x7fff)
+ return -EINVAL;
+
+ rtd119x_rtc_set_enabled(dev, false);
+
+ writel_relaxed((tm->tm_sec << 1) & RTD_RTCSEC_RTCSEC_MASK, data->base + RTD_RTCSEC);
+ writel_relaxed(tm->tm_min & RTD_RTCMIN_RTCMIN_MASK, data->base + RTD_RTCMIN);
+ writel_relaxed(tm->tm_hour & RTD_RTCHR_RTCHR_MASK, data->base + RTD_RTCHR);
+ writel_relaxed(day & RTD_RTCDATE1_RTCDATE1_MASK, data->base + RTD_RTCDATE1);
+ writel_relaxed((day >> 8) & RTD_RTCDATE2_RTCDATE2_MASK, data->base + RTD_RTCDATE2);
+
+ rtd119x_rtc_set_enabled(dev, true);
+
+ return 0;
+}
+
+static const struct rtc_class_ops rtd119x_rtc_ops = {
+ .read_time = rtd119x_rtc_read_time,
+ .set_time = rtd119x_rtc_set_time,
+};
+
+static const struct of_device_id rtd119x_rtc_dt_ids[] = {
+ { .compatible = "realtek,rtd1295-rtc" },
+ { }
+};
+
+static int rtd119x_rtc_probe(struct platform_device *pdev)
+{
+ struct rtd119x_rtc *data;
+ struct resource *res;
+ u32 val;
+ int ret;
+
+ data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, data);
+ data->base_year = 2014;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ data->base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(data->base))
+ return PTR_ERR(data->base);
+
+ data->clk = of_clk_get(pdev->dev.of_node, 0);
+ if (IS_ERR(data->clk))
+ return PTR_ERR(data->clk);
+
+ ret = clk_prepare_enable(data->clk);
+ if (ret) {
+ clk_put(data->clk);
+ return ret;
+ }
+
+ val = readl_relaxed(data->base + RTD_RTCACR);
+ if (!(val & RTD_RTCACR_RTCPWR)) {
+ writel_relaxed(RTD_RTCACR_RTCPWR, data->base + RTD_RTCACR);
+
+ rtd119x_rtc_reset(&pdev->dev);
+
+ writel_relaxed(0, data->base + RTD_RTCMIN);
+ writel_relaxed(0, data->base + RTD_RTCHR);
+ writel_relaxed(0, data->base + RTD_RTCDATE1);
+ writel_relaxed(0, data->base + RTD_RTCDATE2);
+ }
+
+ rtd119x_rtc_set_enabled(&pdev->dev, true);
+
+ data->rtcdev = devm_rtc_device_register(&pdev->dev, "rtc",
+ &rtd119x_rtc_ops, THIS_MODULE);
+ if (IS_ERR(data->rtcdev)) {
+ dev_err(&pdev->dev, "failed to register rtc device");
+ clk_disable_unprepare(data->clk);
+ clk_put(data->clk);
+ return PTR_ERR(data->rtcdev);
+ }
+
+ return 0;
+}
+
+static int rtd119x_rtc_remove(struct platform_device *pdev)
+{
+ struct rtd119x_rtc *data = platform_get_drvdata(pdev);
+
+ rtd119x_rtc_set_enabled(&pdev->dev, false);
+
+ clk_disable_unprepare(data->clk);
+
+ return 0;
+}
+
+static struct platform_driver rtd119x_rtc_driver = {
+ .probe = rtd119x_rtc_probe,
+ .remove = rtd119x_rtc_remove,
+ .driver = {
+ .name = "rtd1295-rtc",
+ .of_match_table = rtd119x_rtc_dt_ids,
+ },
+};
+builtin_platform_driver(rtd119x_rtc_driver);
--
2.12.3
^ permalink raw reply related
* [PATCH v3 1/3] dt-bindings: rtc: Add Realtek RTD1295
From: Andreas Färber @ 2017-08-28 11:10 UTC (permalink / raw)
To: Alessandro Zummo, Alexandre Belloni, linux-rtc, linux-arm-kernel
Cc: linux-kernel, Roc He, 蒋丽琴,
Andreas Färber, Rob Herring, Mark Rutland, devicetree
In-Reply-To: <20170828111041.17946-1-afaerber@suse.de>
Add a binding for the RTC on the Realtek RTD119x/RTD129x SoC families.
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
v1 -> v2 -> v3: Unchanged
.../devicetree/bindings/rtc/realtek,rtd119x.txt | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
create mode 100644 Documentation/devicetree/bindings/rtc/realtek,rtd119x.txt
diff --git a/Documentation/devicetree/bindings/rtc/realtek,rtd119x.txt b/Documentation/devicetree/bindings/rtc/realtek,rtd119x.txt
new file mode 100644
index 000000000000..bbf1ccb5df31
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/realtek,rtd119x.txt
@@ -0,0 +1,16 @@
+Realtek RTD129x Real-Time Clock
+===============================
+
+Required properties:
+- compatible : Should be "realtek,rtd1295-rtc"
+- reg : Specifies the physical base address and size
+- clocks : Specifies the clock gate
+
+
+Example:
+
+ rtc@9801b600 {
+ compatible = "realtek,rtd1295-clk";
+ reg = <0x9801b600 0x100>;
+ clocks = <&clkc RTD1295_CLK_EN_MISC_RTC>;
+ };
--
2.12.3
^ permalink raw reply related
* [PATCH v3 0/3] arm64: Realtek RTD1295 RTC
From: Andreas Färber @ 2017-08-28 11:10 UTC (permalink / raw)
To: Alessandro Zummo, Alexandre Belloni, linux-rtc, linux-arm-kernel
Cc: linux-kernel, Roc He, 蒋丽琴,
Andreas Färber, devicetree, Andrew Lunn
Hello,
This series adds the RTC for the Realtek RTD1295 SoC.
Based on my RTD1295 clk series.
There being no public source code for RTD1295, the implementation is based on
register offsets seen in the vendor DT, as well as older mach-rtk119x code
published by QNAP.
v3 drops a redundant spinlock and refactors a helper.
The DT node depends on the clk series for clock index and header.
More experimental patches at:
https://github.com/afaerber/linux/commits/rtd1295-next
Have a lot of fun!
Cheers,
Andreas
v2 -> v3:
* Dropped spinlock (Andrew)
* Improved year_days vs. days_in_year readability
v1 -> v2:
* Updated rtc driver to no longer use open/release (Alexandre)
* Cleaned up debug output (Andrew)
* Avoided COMPILE_TEST division errors (kbuild)
* Various cleanups and extensions
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: linux-rtc@vger.kernel.org
Cc: Roc He <hepeng@zidoo.tv>
Cc: 蒋丽琴 <jiang.liqin@geniatech.com>
Cc: devicetree@vger.kernel.org
Cc: Andrew Lunn <andrew@lunn.ch>
Andreas Färber (3):
dt-bindings: rtc: Add Realtek RTD1295
rtc: Add Realtek RTD1295
arm64: dts: realtek: Add RTD1295 RTC node
.../devicetree/bindings/rtc/realtek,rtd119x.txt | 16 ++
arch/arm64/boot/dts/realtek/rtd1295.dtsi | 6 +
drivers/rtc/Kconfig | 8 +
drivers/rtc/Makefile | 1 +
drivers/rtc/rtc-rtd119x.c | 241 +++++++++++++++++++++
5 files changed, 272 insertions(+)
create mode 100644 Documentation/devicetree/bindings/rtc/realtek,rtd119x.txt
create mode 100644 drivers/rtc/rtc-rtd119x.c
--
2.12.3
^ permalink raw reply
* Re: [PATCH] rtc: core: let rtc_set_time properly populate element wday
From: Alexandre Belloni @ 2017-08-28 7:54 UTC (permalink / raw)
To: Heiner Kallweit; +Cc: linux-rtc
In-Reply-To: <7da740e6-b1e0-0a58-8b54-2e5e3e2f6c54@gmail.com>
On 27/08/2017 at 21:35:34 +0200, Heiner Kallweit wrote:
> When having a look at the MCP794XX datasheet it became clear why the
> mentioned weekday-related patch was submitted. This chip is quite
> strange regarding the possible alarm match conditions:
>
> ALMxMSK<2:0>: Alarm Mask bits
> 000 = Seconds match
> 001 = Minutes match
> 010 = Hours match (logic takes into account 12-/24-hour operation)
> 011 = Day of week match
> 100 = Date match
> 101 = Reserved; do not use
> 110 = Reserved; do not use
> 111 = Seconds, Minutes, Hour, Day of Week, Date and Month
>
> When not having a proper weekday you get only either seconds or
> minutes or hours or date match.
>
> So it's best to ensure that the weekday is properly populated.
> However we can do this in the driver and don't have to touch the core.
>
Can you do that only for mcp794xx? I received one earlier this month and
I'll separate the driver from rtc-ds1307.c at some point.
BTW, I've rebased and pushed some cleanup for this driver in:
https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git/log/?h=rtc-ds1307
Can you base yourself on that?
Thanks,
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH] rtc: core: let rtc_set_time properly populate element wday
From: Heiner Kallweit @ 2017-08-27 19:35 UTC (permalink / raw)
To: Alexandre Belloni; +Cc: linux-rtc
In-Reply-To: <a6ccd0ab-a6f4-1122-233b-61e69b80aa54@gmail.com>
Am 27.08.2017 um 14:39 schrieb Heiner Kallweit:
> Am 26.08.2017 um 21:59 schrieb Alexandre Belloni:
>> On 26/08/2017 at 21:56:07 +0200, Alexandre Belloni wrote:
>>> There is no point in fixing that. Nobody uses wday.
>>>
>>
>> And that was going to be my comment on your other patch. You may as well
>> just remove the whole wday correction from ds1307.
>>
> Actually I was wondering too when wday should ever be used, except
> theoretically for an alarm, however commit e29385fab0bf
> "rtc: ds1307: Fix relying on reset value for weekday" was added just
> a year ago.
>
When having a look at the MCP794XX datasheet it became clear why the
mentioned weekday-related patch was submitted. This chip is quite
strange regarding the possible alarm match conditions:
ALMxMSK<2:0>: Alarm Mask bits
000 = Seconds match
001 = Minutes match
010 = Hours match (logic takes into account 12-/24-hour operation)
011 = Day of week match
100 = Date match
101 = Reserved; do not use
110 = Reserved; do not use
111 = Seconds, Minutes, Hour, Day of Week, Date and Month
When not having a proper weekday you get only either seconds or
minutes or hours or date match.
So it's best to ensure that the weekday is properly populated.
However we can do this in the driver and don't have to touch the core.
> I think then we can remove the usage of wday in general, including
> writing / reading wday in ds1307_get/set_time.
>
>>> On 26/08/2017 at 21:16:34 +0200, Heiner Kallweit wrote:
>>>> Some drivers use element wday of the struct rtc_time which is passed to
>>>> callback set_time. However element wday may incorrectly or not be
>>>> populated if struct rtc_time is coming from userspace via rtc_dev_ioctl.
>>>> rtc_valid_tm() doesn't check element wday.
>>>>
>>>> Therefore convert struct rtc_time to time64_t and then use
>>>> rtc_time64_to_tm to generate a struct rtc_time with all elements properly
>>>> populated.
>>>>
>>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>>> ---
>>>> drivers/rtc/interface.c | 24 ++++++++++++++++--------
>>>> 1 file changed, 16 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
>>>> index 8cec9a02..5a53c590 100644
>>>> --- a/drivers/rtc/interface.c
>>>> +++ b/drivers/rtc/interface.c
>>>> @@ -59,28 +59,36 @@ EXPORT_SYMBOL_GPL(rtc_read_time);
>>>>
>>>> int rtc_set_time(struct rtc_device *rtc, struct rtc_time *tm)
>>>> {
>>>> + time64_t secs64;
>>>> int err;
>>>>
>>>> err = rtc_valid_tm(tm);
>>>> if (err != 0)
>>>> return err;
>>>>
>>>> + secs64 = rtc_tm_to_time64(tm);
>>>> +
>>>> err = mutex_lock_interruptible(&rtc->ops_lock);
>>>> if (err)
>>>> return err;
>>>>
>>>> if (!rtc->ops)
>>>> err = -ENODEV;
>>>> - else if (rtc->ops->set_time)
>>>> - err = rtc->ops->set_time(rtc->dev.parent, tm);
>>>> - else if (rtc->ops->set_mmss64) {
>>>> - time64_t secs64 = rtc_tm_to_time64(tm);
>>>> -
>>>> + else if (rtc->ops->set_time) {
>>>> + struct rtc_time tmp;
>>>> +
>>>> + /*
>>>> + * element wday of tm may not be set, therefore use
>>>> + * rtc_time64_to_tm to generate a struct rtc_time
>>>> + * with all elements being properly populated
>>>> + */
>>>> + rtc_time64_to_tm(secs64, &tmp);
>>>> + err = rtc->ops->set_time(rtc->dev.parent, &tmp);
>>>> + } else if (rtc->ops->set_mmss64)
>>>> err = rtc->ops->set_mmss64(rtc->dev.parent, secs64);
>>>> - } else if (rtc->ops->set_mmss) {
>>>> - time64_t secs64 = rtc_tm_to_time64(tm);
>>>> + else if (rtc->ops->set_mmss)
>>>> err = rtc->ops->set_mmss(rtc->dev.parent, secs64);
>>>> - } else
>>>> + else
>>>> err = -EINVAL;
>>>>
>>>> pm_stay_awake(rtc->dev.parent);
>>>> --
>>>> 2.14.1
>>>>
>>>
>>> --
>>> Alexandre Belloni, Free Electrons
>>> Embedded Linux and Kernel engineering
>>> http://free-electrons.com
>>
>
^ permalink raw reply
* Re: [PATCH v2 2/3] rtc: Add Realtek RTD1295
From: Alexandre Belloni @ 2017-08-27 19:26 UTC (permalink / raw)
To: Andrew Lunn
Cc: Andreas Färber, linux-rtc, Alessandro Zummo, Roc He,
?????????, linux-kernel, linux-arm-kernel
In-Reply-To: <20170827133751.GD13622@lunn.ch>
On 27/08/2017 at 15:37:51 +0200, Andrew Lunn wrote:
> > >> +static inline int rtd119x_rtc_year_days(int year)
> > >> +{
> > >> + return rtc_year_days(1, 12, year);
> > >
> > > I'm not sure it is worth wrapping rtc_year_days
> > [snip]
> >
> > Well, I found your rtc_year_days rather confusing and had to play with
> > the arguments until I got it working as expected, so I wanted an inline
> > function (or macro) as abstraction from my three callers.
>
> I agree with that. I was wondering why 1st December was being used. I
> would say this API does not do too well on Rusty's API Design
> Manifesto.
>
It follows the convention that tm_mon is 0-11. Making it easy to read
for anyone used to the RTC subsystem. I can agree it is not the most
intuitive one. That choice predates the RTC subsystem (made in 1996), I
won't give any name ;)
> It does at least get the day/month/year in the right order ;-)
>
> Andrew
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [RFC 1/3] dt-bindings: rtc: Add Realtek RTD1295
From: Alexandre Belloni @ 2017-08-27 19:07 UTC (permalink / raw)
To: Andreas Färber
Cc: Andrew Lunn, Rob Herring, linux-rtc, Alessandro Zummo, Roc He,
devicetree, ?????????, linux-kernel, Mark Rutland,
linux-arm-kernel
In-Reply-To: <feaa47ff-083a-9ee5-c055-ff1f7efce371@suse.de>
On 27/08/2017 at 19:26:11 +0200, Andreas Färber wrote:
> Hi Andrew,
>
> Am 27.08.2017 um 15:47 schrieb Andrew Lunn:
> >> Thanks. Did you read the RFC question in the cover letter as well and
> >> have any comments? Downstream has an rtc-base-year = <2014>; property
> >> that I had left out in this RFC and due to your ack not included in v2.
> >>
> >> Should we default to 2014 in the driver and add an optional base-year
> >> property once we encounter a diverging device, or should we make it
> >> required from the beginning? I did not spot any other rtc binding with
> >> such a property and would appreciate a clarification.
> >
> > From the perspective of the hardware, does it care what the base is?
>
> The hardware stores a 15-bit number of days since Jan 1st of that base
> year. It does not store the base year.
>
> The datasheet does not name such a base year. No manual is available.
>
> The driver needs to get it from somewhere for calculating day/month/year
> in read_time and days in set_time.
>
> The read_offset/set_offset API appeared to be something different.
>
There is no api to change the epoch of an RTC because it is difficult to
do in a race free way. This has to be worked on.
Anyway, you don't really care for now as it goes up to 2093 and
hopefully, you will never have a product with a different epoch.
> > A device using a different base will initially return the wrong
> > time. But once the correct time has been written back, it will be O.K.
> >
The other issue being that you don't have any way to know whether the
base is correct or not until the date is set.
> > This only becomes an issue if a device is used with different OSs,
> > which have different bases. Swapping back and forth between OSs then
> > becomes an issue.
>
> These are TV boxes, so yes, I'm dual-booting into Android with a vendor
> 4.1 kernel and would like to keep date compatibility.
>
> https://en.opensuse.org/HCL:Zidoo_X9S
> https://en.opensuse.org/HCL:ProBox2_Ava
> https://en.opensuse.org/HCL:Lake1
>
> As stated in the v2 rtc commit message, 2014 is the base year
> encountered on all three devices that I've had access to.
> @Jiang, if you're using a different base year, please speak up!
>
> > KISS suggests not having a base in DT until it is actually
> > required. Since it is an additional property, it does not break
> > backwards compatibility when added.
>
> That's what I've attempted here - but for RDA8810PL serial Rob said he
> did not want future changes to my binding, therefore I am asking for his
> confirmation here.
It would be a change that is easy enough to do in a backward compatible
way so that is probably fine.
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [RFC 1/3] dt-bindings: rtc: Add Realtek RTD1295
From: Andreas Färber @ 2017-08-27 17:26 UTC (permalink / raw)
To: Andrew Lunn
Cc: Rob Herring, linux-rtc, Alessandro Zummo, Roc He, devicetree,
?????????, linux-kernel, Alexandre Belloni, Mark Rutland,
linux-arm-kernel
In-Reply-To: <20170827134729.GE13622@lunn.ch>
Hi Andrew,
Am 27.08.2017 um 15:47 schrieb Andrew Lunn:
>> Thanks. Did you read the RFC question in the cover letter as well and
>> have any comments? Downstream has an rtc-base-year = <2014>; property
>> that I had left out in this RFC and due to your ack not included in v2.
>>
>> Should we default to 2014 in the driver and add an optional base-year
>> property once we encounter a diverging device, or should we make it
>> required from the beginning? I did not spot any other rtc binding with
>> such a property and would appreciate a clarification.
>
> From the perspective of the hardware, does it care what the base is?
The hardware stores a 15-bit number of days since Jan 1st of that base
year. It does not store the base year.
The datasheet does not name such a base year. No manual is available.
The driver needs to get it from somewhere for calculating day/month/year
in read_time and days in set_time.
The read_offset/set_offset API appeared to be something different.
> A device using a different base will initially return the wrong
> time. But once the correct time has been written back, it will be O.K.
>
> This only becomes an issue if a device is used with different OSs,
> which have different bases. Swapping back and forth between OSs then
> becomes an issue.
These are TV boxes, so yes, I'm dual-booting into Android with a vendor
4.1 kernel and would like to keep date compatibility.
https://en.opensuse.org/HCL:Zidoo_X9S
https://en.opensuse.org/HCL:ProBox2_Ava
https://en.opensuse.org/HCL:Lake1
As stated in the v2 rtc commit message, 2014 is the base year
encountered on all three devices that I've had access to.
@Jiang, if you're using a different base year, please speak up!
> KISS suggests not having a base in DT until it is actually
> required. Since it is an additional property, it does not break
> backwards compatibility when added.
That's what I've attempted here - but for RDA8810PL serial Rob said he
did not want future changes to my binding, therefore I am asking for his
confirmation here.
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox