Linux RTC
 help / color / mirror / Atom feed
* Re: [PATCH] rtc: sun6i: Add support for the external oscillator gate
From: Alexandre Belloni @ 2017-08-24 14:36 UTC (permalink / raw)
  To: Maxime Ripard; +Cc: Chen-Yu Tsai, linux-rtc, linux-arm-kernel, linux-kernel
In-Reply-To: <20170824121854.23995-1-maxime.ripard@free-electrons.com>

Hi,

On 24/08/2017 at 14:18:54 +0200, Maxime Ripard wrote:
> The RTC can output its 32kHz clock outside of the SoC, for example to clock
> a WiFi chip.
> 
> Create a new clock that other devices will be able to retrieve, while
> maintaining the DT stability by providing a default name for that clock if
> clock-output-names doesn't list one.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  .../devicetree/bindings/rtc/sun6i-rtc.txt          |  4 ++--
>  drivers/rtc/rtc-sun6i.c                            | 24 +++++++++++++++++++---
>  2 files changed, 23 insertions(+), 5 deletions(-)
> 

It doesn't apply cleanly, can you rebase on top of -next?

> diff --git a/Documentation/devicetree/bindings/rtc/sun6i-rtc.txt b/Documentation/devicetree/bindings/rtc/sun6i-rtc.txt
> index 945934918b71..d5e26d313f62 100644
> --- a/Documentation/devicetree/bindings/rtc/sun6i-rtc.txt
> +++ b/Documentation/devicetree/bindings/rtc/sun6i-rtc.txt
> @@ -10,7 +10,7 @@ Required properties:
>  
>  Required properties for new device trees
>  - clocks	: phandle to the 32kHz external oscillator
> -- clock-output-names : name of the LOSC clock created
> +- clock-output-names : names of the LOSC and its external output clocks created
>  - #clock-cells  : must be equals to 1. The RTC provides two clocks: the
>  		  LOSC and its external output, with index 0 and 1
>  		  respectively.
> @@ -21,7 +21,7 @@ rtc: rtc@01f00000 {
>  	compatible = "allwinner,sun6i-a31-rtc";
>  	reg = <0x01f00000 0x54>;
>  	interrupts = <0 40 4>, <0 41 4>;
> -	clock-output-names = "osc32k";
> +	clock-output-names = "osc32k", "osc32k-out";
>  	clocks = <&ext_osc32k>;
>  	#clock-cells = <1>;
>  };
> diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c
> index 39cbc1238b92..d4a6ddbfd872 100644
> --- a/drivers/rtc/rtc-sun6i.c
> +++ b/drivers/rtc/rtc-sun6i.c
> @@ -73,6 +73,9 @@
>  #define SUN6I_ALARM_CONFIG			0x0050
>  #define SUN6I_ALARM_CONFIG_WAKEUP		BIT(0)
>  
> +#define SUN6I_LOSC_OUT_GATING			0x0060
> +#define SUN6I_LOSC_OUT_GATING_EN		BIT(0)
> +
>  /*
>   * Get date values
>   */
> @@ -125,6 +128,7 @@ struct sun6i_rtc_dev {
>  	struct clk_hw hw;
>  	struct clk_hw *int_osc;
>  	struct clk *losc;
> +	struct clk *ext_losc;
>  
>  	spinlock_t lock;
>  };
> @@ -188,6 +192,7 @@ static void __init sun6i_rtc_clk_init(struct device_node *node)
>  	struct clk_init_data init = {
>  		.ops		= &sun6i_rtc_osc_ops,
>  	};
> +	const char *clkout_name = "osc32k-out";
>  	const char *parents[2];
>  
>  	rtc = kzalloc(sizeof(*rtc), GFP_KERNEL);
> @@ -195,7 +200,7 @@ static void __init sun6i_rtc_clk_init(struct device_node *node)
>  		return;
>  	spin_lock_init(&rtc->lock);
>  
> -	clk_data = kzalloc(sizeof(*clk_data) + sizeof(*clk_data->hws),
> +	clk_data = kzalloc(sizeof(*clk_data) + (sizeof(*clk_data->hws) * 2),
>  			   GFP_KERNEL);
>  	if (!clk_data)
>  		return;
> @@ -235,7 +240,8 @@ static void __init sun6i_rtc_clk_init(struct device_node *node)
>  
>  	init.parent_names = parents;
>  	init.num_parents = of_clk_get_parent_count(node) + 1;
> -	of_property_read_string(node, "clock-output-names", &init.name);
> +	of_property_read_string_index(node, "clock-output-names", 0,
> +				      &init.name);
>  
>  	rtc->losc = clk_register(NULL, &rtc->hw);
>  	if (IS_ERR(rtc->losc)) {
> @@ -243,8 +249,20 @@ static void __init sun6i_rtc_clk_init(struct device_node *node)
>  		return;
>  	}
>  
> -	clk_data->num = 1;
> +	of_property_read_string_index(node, "clock-output-names", 1,
> +				      &clkout_name);
> +	rtc->ext_losc = clk_register_gate(NULL, clkout_name, rtc->hw.init->name,
> +					  0, rtc->base + SUN6I_LOSC_OUT_GATING,
> +					  SUN6I_LOSC_OUT_GATING_EN, 0,
> +					  &rtc->lock);
> +	if (IS_ERR(rtc->ext_losc)) {
> +		pr_crit("Couldn't register the LOSC external gate\n");
> +		return;
> +	}
> +
> +	clk_data->num = 2;
>  	clk_data->hws[0] = &rtc->hw;
> +	clk_data->hws[1] = __clk_get_hw(rtc->ext_losc);
>  	of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
>  }
>  CLK_OF_DECLARE_DRIVER(sun6i_rtc_clk, "allwinner,sun6i-a31-rtc",
> -- 
> 2.13.5
> 

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* Re: rtc: ds1307: call the platform's logic for handle IRQs.
From: Heiner Kallweit @ 2017-08-24 18:46 UTC (permalink / raw)
  To: Alexandre Belloni, enric.balletbo; +Cc: linux-rtc

Hi Enric,

I just saw your submitted patch on patchwork. As I haven't been subscribed
to linux-rtc list yet, I can't reply to the original mail.

Few remarks:
I think the same can be achieved easier (apart from the fact that member
irq was just removed from struct ds1307).
The curent call to device_set_wakeup_capable has to be replaced with
device_init_wakeup, in addition we have to call dev_pm_set_wake_irq to
register the interrupt with the Linux wakeup core. Then the core takes
care of everything.

See also rtc-ds1343, although I think the calls to enable/disable_irq_wake
are not needed there because the core takes care of this already
(enable_irq_wake is called from dev_pm_arm_wake_irq).

Rgds, Heiner

^ permalink raw reply

* Re: rtc: ds1307: call the platform's logic for handle IRQs.
From: Heiner Kallweit @ 2017-08-24 19:16 UTC (permalink / raw)
  To: Alexandre Belloni, enric.balletbo; +Cc: linux-rtc
In-Reply-To: <832da9eb-abed-67c9-1d53-34f19d27c0d9@gmail.com>

Am 24.08.2017 um 20:46 schrieb Heiner Kallweit:
> Hi Enric,
> 
> I just saw your submitted patch on patchwork. As I haven't been subscribed
> to linux-rtc list yet, I can't reply to the original mail.
> 
> Few remarks:
> I think the same can be achieved easier (apart from the fact that member
> irq was just removed from struct ds1307).
> The curent call to device_set_wakeup_capable has to be replaced with
> device_init_wakeup, in addition we have to call dev_pm_set_wake_irq to
> register the interrupt with the Linux wakeup core. Then the core takes
> care of everything.
> 
After further checking not even this may be necessary.
If flag I2C_CLIENT_WAKE is set for the i2c client then i2c_device_probe()
enables the device as wakeup device and registers the interrupt with the
wakeup core.
If the i2c client is defined via DT, then of_i2c_register_device() sets
this flag based on the "wakeup-source" property.
Is your device configured via DT? If yes, did you check whether wakeup
works w/o your patch with just setting property wakeup-source ?


> See also rtc-ds1343, although I think the calls to enable/disable_irq_wake
> are not needed there because the core takes care of this already
> (enable_irq_wake is called from dev_pm_arm_wake_irq).
> 
> Rgds, Heiner
> 

^ permalink raw reply

* Re: rtc: ds1307: call the platform's logic for handle IRQs.
From: Enric Balletbo i Serra @ 2017-08-24 20:13 UTC (permalink / raw)
  To: Heiner Kallweit, Alexandre Belloni; +Cc: linux-rtc
In-Reply-To: <019f5769-b3ec-2afd-9469-e722b7b0a2e5@gmail.com>

Hi Heiner

On 24/08/17 21:16, Heiner Kallweit wrote:
> Am 24.08.2017 um 20:46 schrieb Heiner Kallweit:
>> Hi Enric,
>>
>> I just saw your submitted patch on patchwork. As I haven't been subscribed
>> to linux-rtc list yet, I can't reply to the original mail.
>>
>> Few remarks:
>> I think the same can be achieved easier (apart from the fact that member
>> irq was just removed from struct ds1307).
>> The curent call to device_set_wakeup_capable has to be replaced with
>> device_init_wakeup, in addition we have to call dev_pm_set_wake_irq to
>> register the interrupt with the Linux wakeup core. Then the core takes
>> care of everything.
>>

Oh I just saw your patches, thanks to advice me about them.

> After further checking not even this may be necessary.
> If flag I2C_CLIENT_WAKE is set for the i2c client then i2c_device_probe()
> enables the device as wakeup device and registers the interrupt with the
> wakeup core.
> If the i2c client is defined via DT, then of_i2c_register_device() sets
> this flag based on the "wakeup-source" property.
> Is your device configured via DT? If yes, did you check whether wakeup
> works w/o your patch with just setting property wakeup-source ?
> 

Yes, device is configured via DT and without my patch, just setting property
wakeup-source didn't work, the reason is that code:

http://elixir.free-electrons.com/linux/v4.13-rc6/source/drivers/rtc/rtc-ds1307.c#L1382

I didn't test on top of your patches so let me rebase/rethink my patch on top of
yours.

> 
>> See also rtc-ds1343, although I think the calls to enable/disable_irq_wake
>> are not needed there because the core takes care of this already
>> (enable_irq_wake is called from dev_pm_arm_wake_irq).
>>
>> Rgds, Heiner
>>
> 

Thanks,
 Enric

^ permalink raw reply

* Re: rtc: ds1307: call the platform's logic for handle IRQs.
From: Heiner Kallweit @ 2017-08-24 20:44 UTC (permalink / raw)
  To: Enric Balletbo i Serra, Alexandre Belloni; +Cc: linux-rtc
In-Reply-To: <b5bed7f5-1963-74fb-dba6-dc465f357645@collabora.com>

Am 24.08.2017 um 22:13 schrieb Enric Balletbo i Serra:
> Hi Heiner
> 
> On 24/08/17 21:16, Heiner Kallweit wrote:
>> Am 24.08.2017 um 20:46 schrieb Heiner Kallweit:
>>> Hi Enric,
>>>
>>> I just saw your submitted patch on patchwork. As I haven't been subscribed
>>> to linux-rtc list yet, I can't reply to the original mail.
>>>
>>> Few remarks:
>>> I think the same can be achieved easier (apart from the fact that member
>>> irq was just removed from struct ds1307).
>>> The curent call to device_set_wakeup_capable has to be replaced with
>>> device_init_wakeup, in addition we have to call dev_pm_set_wake_irq to
>>> register the interrupt with the Linux wakeup core. Then the core takes
>>> care of everything.
>>>
> 
> Oh I just saw your patches, thanks to advice me about them.
> 
>> After further checking not even this may be necessary.
>> If flag I2C_CLIENT_WAKE is set for the i2c client then i2c_device_probe()
>> enables the device as wakeup device and registers the interrupt with the
>> wakeup core.
>> If the i2c client is defined via DT, then of_i2c_register_device() sets
>> this flag based on the "wakeup-source" property.
>> Is your device configured via DT? If yes, did you check whether wakeup
>> works w/o your patch with just setting property wakeup-source ?
>>
> 
> Yes, device is configured via DT and without my patch, just setting property
> wakeup-source didn't work, the reason is that code:
> 
> http://elixir.free-electrons.com/linux/v4.13-rc6/source/drivers/rtc/rtc-ds1307.c#L1382
> 
This shouldn't be the reason because registering the interrupt with the wakeup core
happens earlier in i2c_device_probe().
Can you check whether dev_pm_set_wake_irq() is actually called with the right
irq number in i2c_device_probe()?

Would be interesting to see the DT config of your rtc.

> I didn't test on top of your patches so let me rebase/rethink my patch on top of
> yours.
> 
>>
>>> See also rtc-ds1343, although I think the calls to enable/disable_irq_wake
>>> are not needed there because the core takes care of this already
>>> (enable_irq_wake is called from dev_pm_arm_wake_irq).
>>>
>>> Rgds, Heiner
>>>
>>
> 
> Thanks,
>  Enric
> 

^ permalink raw reply

* Re: rtc: ds1307: call the platform's logic for handle IRQs.
From: Heiner Kallweit @ 2017-08-24 20:53 UTC (permalink / raw)
  To: Enric Balletbo i Serra, Alexandre Belloni; +Cc: linux-rtc
In-Reply-To: <04725acb-b57f-a303-1be6-2b5d74396f96@gmail.com>

Am 24.08.2017 um 22:44 schrieb Heiner Kallweit:
> Am 24.08.2017 um 22:13 schrieb Enric Balletbo i Serra:
>> Hi Heiner
>>
>> On 24/08/17 21:16, Heiner Kallweit wrote:
>>> Am 24.08.2017 um 20:46 schrieb Heiner Kallweit:
>>>> Hi Enric,
>>>>
>>>> I just saw your submitted patch on patchwork. As I haven't been subscribed
>>>> to linux-rtc list yet, I can't reply to the original mail.
>>>>
>>>> Few remarks:
>>>> I think the same can be achieved easier (apart from the fact that member
>>>> irq was just removed from struct ds1307).
>>>> The curent call to device_set_wakeup_capable has to be replaced with
>>>> device_init_wakeup, in addition we have to call dev_pm_set_wake_irq to
>>>> register the interrupt with the Linux wakeup core. Then the core takes
>>>> care of everything.
>>>>
>>
>> Oh I just saw your patches, thanks to advice me about them.
>>
>>> After further checking not even this may be necessary.
>>> If flag I2C_CLIENT_WAKE is set for the i2c client then i2c_device_probe()
>>> enables the device as wakeup device and registers the interrupt with the
>>> wakeup core.
>>> If the i2c client is defined via DT, then of_i2c_register_device() sets
>>> this flag based on the "wakeup-source" property.
>>> Is your device configured via DT? If yes, did you check whether wakeup
>>> works w/o your patch with just setting property wakeup-source ?
>>>
>>
>> Yes, device is configured via DT and without my patch, just setting property
>> wakeup-source didn't work, the reason is that code:
>>
>> http://elixir.free-electrons.com/linux/v4.13-rc6/source/drivers/rtc/rtc-ds1307.c#L1382
>>
> This shouldn't be the reason because registering the interrupt with the wakeup core
> happens earlier in i2c_device_probe().
> Can you check whether dev_pm_set_wake_irq() is actually called with the right
> irq number in i2c_device_probe()?
> 
See also description of commit 4990d4fe327b9d9a7a
"PM / Wakeirq: Add automated device wake IRQ handling" from 2015.
It explains why the code you want to add shouldn't be needed any longer.

> Would be interesting to see the DT config of your rtc.
> 
>> I didn't test on top of your patches so let me rebase/rethink my patch on top of
>> yours.
>>
>>>
>>>> See also rtc-ds1343, although I think the calls to enable/disable_irq_wake
>>>> are not needed there because the core takes care of this already
>>>> (enable_irq_wake is called from dev_pm_arm_wake_irq).
>>>>
>>>> Rgds, Heiner
>>>>
>>>
>>
>> Thanks,
>>  Enric
>>
> 

^ permalink raw reply

* Re: [PATCH 0/9] rtc: ds1307: series with refactorings / improvements
From: Alexandre Belloni @ 2017-08-24 21:04 UTC (permalink / raw)
  To: Heiner Kallweit; +Cc: linux-rtc
In-Reply-To: <aded2895-cf4b-12f0-5525-c4389ccf3650@gmail.com>

Hi,

On 12/07/2017 at 07:23:31 +0200, Heiner Kallweit wrote:
> This series includes smaller refactorings and improvements.
> 
> Heiner Kallweit (9):
>   rtc: ds1307: remove member irq from struct ds1307
>   rtc: ds1307: factor out bbsqi bit to struct chip_desc
>   rtc: ds1307: improve trickle charger initialization
>   rtc: ds1307: constify struct chip_desc variables
>   rtc: ds1307: improve irq setup
>   rtc: ds1307: factor out irq_handler to struct chip_desc
>   rtc: ds1307: factor out rtc_ops to struct chip_desc
>   rtc: ds1307: factor out offset to struct chip_desc
>   rtc: ds1307: remove member nvram_offset from struct ds1307
> 
>  drivers/rtc/rtc-ds1307.c | 165 ++++++++++++++++++++++-------------------------
>  1 file changed, 76 insertions(+), 89 deletions(-)
> 

I finally reviewed and applied the series. I had to fix a few whitespace
issues though.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* Re: [PATCH v2] rtc: ds1307: add basic support for ds1341 chip
From: Alexandre Belloni @ 2017-08-24 21:19 UTC (permalink / raw)
  To: Nikita Yushchenko
  Cc: Alessandro Zummo, Heiner Kallweit, Linus Walleij, Arnaud Ebalard,
	David Lowe, Javier Martinez Canillas, Marek Vasut, Tin Huynh,
	linux-rtc, linux-kernel, Andrey Smirnov, Aleksander Morgado,
	Chris Healy
In-Reply-To: <20170824063211.27635-1-nikita.yoush@cogentembedded.com>

Hi,

On 24/08/2017 at 09:32:11 +0300, Nikita Yushchenko wrote:
> This adds support for reading and writing date/time from/to ds1341 chip.
> 
> ds1341 chip has other features - alarms, input clock (can be used instead
> of intercal oscillator for better accuracy), output clock ("square wave
> generation"). However, not all of that is available at the same time.
> Same chip pins, CLKIN/nINTA and SQW/nINTB, can be used either for
> input/output clocks, or for alarm interrupts. Role of these pins on
> particular board depends on hardware wiring.
> 
> We can add device tree properties that describe if each of pins is wired
> as clock, or as interrupt, or left unconnected, and enable support for
> corresponding functionality based on that. But that is cumbersome, requires
> hardware for testing, and has to deal with bit enabling/disabling output
> clock also affects which pins alarm interrupts are routed to.
> 
> Another factor is that there are hardware setups (i.e. ZII RDU2) that
> power DS1341 from SuperCap, which makes power saving critical. For such
> setups, kernel driver should leave register bits that control mentioned
> pins in the state configured by bootloader.
> 
> Given all that, it was decided to limit support to "only date/time" for
> now. That is enough for common use case. Full (and cumbersome)
> implementation can be added later if ever needed.
> 
> Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Tested-by: Aleksander Morgado <aleksander@aleksander.es>
> ---
>  drivers/rtc/Kconfig      | 10 +++++-----
>  drivers/rtc/rtc-ds1307.c | 13 +++++++++++++
>  2 files changed, 18 insertions(+), 5 deletions(-)

I've applied but...

> @@ -1323,6 +1334,7 @@ static int ds1307_probe(struct i2c_client *client,
>  	static const int	bbsqi_bitpos[] = {
>  		[ds_1337] = 0,
>  		[ds_1339] = DS1339_BIT_BBSQI,
> +		[ds_1341] = 0,
>  		[ds_3231] = DS3231_BIT_BBSQW,
>  	};
>  	const struct rtc_class_ops *rtc_ops = &ds13xx_rtc_ops;

I had to remove that change following http://patchwork.ozlabs.org/patch/787009/

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* [PATCH v3 5/5] mfd: mt6397: Add PMIC keys support to MT6397 driver
From: Chen Zhong @ 2017-08-25  6:32 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, Chen Zhong, Linus Walleij, Jaechul Lee,
	linux-input, devicetree, linux-arm-kernel, linux-mediatek,
	linux-kernel, linux-rtc
In-Reply-To: <1503642753-12385-1-git-send-email-chen.zhong@mediatek.com>

This patch adds compatible strings and interrupts for pmic keys
which serves as child device of MFD.

Signed-off-by: Chen Zhong <chen.zhong@mediatek.com>
---
 drivers/mfd/mt6397-core.c |   22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c
index 6546d7f..77b64bd 100644
--- a/drivers/mfd/mt6397-core.c
+++ b/drivers/mfd/mt6397-core.c
@@ -43,6 +43,16 @@
 	},
 };
 
+static const struct resource mt6323_keys_resources[] = {
+	DEFINE_RES_IRQ(MT6323_IRQ_STATUS_PWRKEY),
+	DEFINE_RES_IRQ(MT6323_IRQ_STATUS_FCHRKEY),
+};
+
+static const struct resource mt6397_keys_resources[] = {
+	DEFINE_RES_IRQ(MT6397_IRQ_PWRKEY),
+	DEFINE_RES_IRQ(MT6397_IRQ_HOMEKEY),
+};
+
 static const struct mfd_cell mt6323_devs[] = {
 	{
 		.name = "mt6323-regulator",
@@ -50,6 +60,11 @@
 	}, {
 		.name = "mt6323-led",
 		.of_compatible = "mediatek,mt6323-led"
+	}, {
+		.name = "mtk-pmic-keys",
+		.num_resources = ARRAY_SIZE(mt6323_keys_resources),
+		.resources = mt6323_keys_resources,
+		.of_compatible = "mediatek,mt6323-keys"
 	},
 };
 
@@ -71,7 +86,12 @@
 	}, {
 		.name = "mt6397-pinctrl",
 		.of_compatible = "mediatek,mt6397-pinctrl",
-	},
+	}, {
+		.name = "mtk-pmic-keys",
+		.num_resources = ARRAY_SIZE(mt6397_keys_resources),
+		.resources = mt6397_keys_resources,
+		.of_compatible = "mediatek,mt6397-keys"
+	}
 };
 
 static void mt6397_irq_lock(struct irq_data *data)
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v3 3/5] dt-bindings: mfd: Add bindings for the keys as subnode of PMIC
From: Chen Zhong @ 2017-08-25  6:32 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, Chen Zhong, Linus Walleij, Jaechul Lee,
	linux-input, devicetree, linux-arm-kernel, linux-mediatek,
	linux-kernel, linux-rtc
In-Reply-To: <1503642753-12385-1-git-send-email-chen.zhong@mediatek.com>

This patch adds documentation for device tree bindings for keys support
as the subnode of MT6397/MT6323 PMIC.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Chen Zhong <chen.zhong@mediatek.com>
---
 Documentation/devicetree/bindings/mfd/mt6397.txt |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/mt6397.txt b/Documentation/devicetree/bindings/mfd/mt6397.txt
index 522a3bb..d1df77f 100644
--- a/Documentation/devicetree/bindings/mfd/mt6397.txt
+++ b/Documentation/devicetree/bindings/mfd/mt6397.txt
@@ -7,6 +7,7 @@ MT6397/MT6323 is a multifunction device with the following sub modules:
 - GPIO
 - Clock
 - LED
+- Keys
 
 It is interfaced to host controller using SPI interface by a proprietary hardware
 called PMIC wrapper or pwrap. MT6397/MT6323 MFD is a child device of pwrap.
@@ -40,6 +41,11 @@ Optional subnodes:
 		- compatible: "mediatek,mt6323-led"
 	see Documentation/devicetree/bindings/leds/leds-mt6323.txt
 
+- keys
+	Required properties:
+		- compatible: "mediatek,mt6397-keys" or "mediatek,mt6323-keys"
+	see Documentation/devicetree/bindings/input/mtk-pmic-keys.txt
+
 Example:
 	pwrap: pwrap@1000f000 {
 		compatible = "mediatek,mt8135-pwrap";
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v3 2/5] dt-bindings: input: Add document bindings for mtk-pmic-keys
From: Chen Zhong @ 2017-08-25  6:32 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, Chen Zhong, Linus Walleij, Jaechul Lee,
	linux-input, devicetree, linux-arm-kernel, linux-mediatek,
	linux-kernel, linux-rtc
In-Reply-To: <1503642753-12385-1-git-send-email-chen.zhong@mediatek.com>

This patch adds the device tree binding documentation for the MediaTek
pmic keys found on PMIC MT6397/MT6323.

Signed-off-by: Chen Zhong <chen.zhong@mediatek.com>
---
 .../devicetree/bindings/input/mtk-pmic-keys.txt    |   38 ++++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/mtk-pmic-keys.txt

diff --git a/Documentation/devicetree/bindings/input/mtk-pmic-keys.txt b/Documentation/devicetree/bindings/input/mtk-pmic-keys.txt
new file mode 100644
index 0000000..100ec44
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/mtk-pmic-keys.txt
@@ -0,0 +1,38 @@
+MediaTek MT6397/MT6323 PMIC Keys Device Driver
+
+There are two key functions provided by MT6397/MT6323 PMIC, pwrkey
+and homekey. The key functions are defined as the subnode of the function
+node provided by MT6397/MT6323 PMIC that is being defined as one kind
+of Muti-Function Device (MFD)
+
+For MT6397/MT6323 MFD bindings see:
+Documentation/devicetree/bindings/mfd/mt6397.txt
+
+Required properties:
+- compatible: "mediatek,mt6397-keys" or "mediatek,mt6323-keys"
+- linux,keycodes: Specifies the numeric keycode values to
+	be used for reporting keys presses. The array can
+	contain up to 2 entries.
+
+Optional Properties:
+- wakeup-source: each key can be used as a wakeup source.
+- mediatek,long-press-mode: Long press key shutdown setting, 1 for
+	pwrkey only, 2 for pwrkey/homekey together, others for disabled.
+- debounce-interval: Long press key shutdown debouncing interval time
+	in seconds. 0/1/2/3 for 8/11/14/5 seconds. If not specified defaults to 0.
+
+Example:
+
+	pmic: mt6397 {
+		compatible = "mediatek,mt6397";
+
+		...
+
+		mt6397keys: mt6397keys {
+			compatible = "mediatek,mt6397-keys";
+			linux,keycodes = <KEY_POWER>, <KEY_VOLUMEDOWN>;
+			wakeup-source = <1>, <0>;
+			mediatek,long-press-mode = <1>;
+			debounce-interval = <0>;
+		};
+	};
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v3 4/5] input: Add MediaTek PMIC keys support
From: Chen Zhong @ 2017-08-25  6:32 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, Chen Zhong, Linus Walleij, Jaechul Lee,
	linux-input, devicetree, linux-arm-kernel, linux-mediatek,
	linux-kernel, linux-rtc
In-Reply-To: <1503642753-12385-1-git-send-email-chen.zhong@mediatek.com>

This patch add support to handle MediaTek PMIC MT6397/MT6323 key
interrupts including pwrkey and homekey, also add setting for
long press key shutdown behavior.

Signed-off-by: Chen Zhong <chen.zhong@mediatek.com>
---
 drivers/input/keyboard/Kconfig         |    9 +
 drivers/input/keyboard/Makefile        |    1 +
 drivers/input/keyboard/mtk-pmic-keys.c |  308 ++++++++++++++++++++++++++++++++
 3 files changed, 318 insertions(+)
 create mode 100644 drivers/input/keyboard/mtk-pmic-keys.c

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 4c4ab1c..bd4e20a 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -756,4 +756,13 @@ config KEYBOARD_BCM
 	  To compile this driver as a module, choose M here: the
 	  module will be called bcm-keypad.
 
+config KEYBOARD_MTK_PMIC
+	tristate "MediaTek PMIC keys support"
+	depends on MFD_MT6397
+	help
+	  Say Y here if you want to use the pmic keys (powerkey/homekey).
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called pmic-keys.
+
 endif
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index d2338ba..20c0b98 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -40,6 +40,7 @@ obj-$(CONFIG_KEYBOARD_MATRIX)		+= matrix_keypad.o
 obj-$(CONFIG_KEYBOARD_MAX7359)		+= max7359_keypad.o
 obj-$(CONFIG_KEYBOARD_MCS)		+= mcs_touchkey.o
 obj-$(CONFIG_KEYBOARD_MPR121)		+= mpr121_touchkey.o
+obj-$(CONFIG_KEYBOARD_MTK_PMIC) 	+= mtk-pmic-keys.o
 obj-$(CONFIG_KEYBOARD_NEWTON)		+= newtonkbd.o
 obj-$(CONFIG_KEYBOARD_NOMADIK)		+= nomadik-ske-keypad.o
 obj-$(CONFIG_KEYBOARD_NSPIRE)		+= nspire-keypad.o
diff --git a/drivers/input/keyboard/mtk-pmic-keys.c b/drivers/input/keyboard/mtk-pmic-keys.c
new file mode 100644
index 0000000..40518ce
--- /dev/null
+++ b/drivers/input/keyboard/mtk-pmic-keys.c
@@ -0,0 +1,308 @@
+/*
+ * Copyright (C) 2017 MediaTek, Inc.
+ *
+ * Author: Chen Zhong <chen.zhong@mediatek.com>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/regmap.h>
+#include <linux/mfd/mt6323/registers.h>
+#include <linux/mfd/mt6397/registers.h>
+#include <linux/mfd/mt6397/core.h>
+
+#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
+
+struct 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)	\
+{									\
+	.deb_reg		= _deb_reg,				\
+	.deb_mask		= _deb_mask,				\
+	.intsel_reg		= _intsel_reg,				\
+	.intsel_mask		= _intsel_mask,				\
+}
+
+struct pmic_regs {
+	const struct pmic_keys_regs keys_regs[PMIC_MAX_KEY_COUNT];
+	u32 pmic_rst_reg;
+};
+
+static const struct pmic_regs mt6397_regs = {
+	.keys_regs[PMIC_PWRKEY_INDEX] = PMIC_KEYS_REGS(MT6397_CHRSTATUS,
+		0x8, MT6397_INT_RSV, 0x10),
+	.keys_regs[PMIC_HOMEKEY_INDEX] = PMIC_KEYS_REGS(MT6397_OCSTATUS2,
+		0x10, MT6397_INT_RSV, 0x8),
+	.pmic_rst_reg = MT6397_TOP_RST_MISC,
+};
+
+static const struct pmic_regs mt6323_regs = {
+	.keys_regs[PMIC_PWRKEY_INDEX] = PMIC_KEYS_REGS(MT6323_CHRSTATUS,
+		0x2, MT6323_INT_MISC_CON, 0x10),
+	.keys_regs[PMIC_HOMEKEY_INDEX] = PMIC_KEYS_REGS(MT6323_CHRSTATUS,
+		0x4, MT6323_INT_MISC_CON, 0x8),
+	.pmic_rst_reg = MT6323_TOP_RST_MISC,
+};
+
+struct pmic_keys_info {
+	struct mtk_pmic_keys *keys;
+	const struct pmic_keys_regs *regs;
+	unsigned int keycode;
+	int irq;
+	bool wakeup:1;
+};
+
+struct mtk_pmic_keys {
+	struct input_dev *input_dev;
+	struct device *dev;
+	struct regmap *regmap;
+	struct pmic_keys_info keys[PMIC_MAX_KEY_COUNT];
+};
+
+enum long_press_mode {
+	LP_DISABLE,
+	LP_ONEKEY,
+	LP_TWOKEY,
+};
+
+static void long_press_reset_setup(struct mtk_pmic_keys *keys, u32 pmic_rst_reg)
+{
+	int ret;
+	u32 long_press_mode, long_press_debounce;
+
+	ret = of_property_read_u32(keys->dev->of_node,
+		"debounce-interval", &long_press_debounce);
+	if (ret)
+		long_press_debounce = 0;
+
+	regmap_update_bits(keys->regmap, pmic_rst_reg,
+			   RST_DU_MASK << RST_DU_SHIFT,
+			   long_press_debounce << RST_DU_SHIFT);
+
+	ret = of_property_read_u32(keys->dev->of_node,
+		"mediatek,long-press-mode", &long_press_mode);
+	if (ret)
+		long_press_mode = LP_DISABLE;
+
+	switch (long_press_mode) {
+	case LP_ONEKEY:
+		regmap_update_bits(keys->regmap, pmic_rst_reg,
+				   PWRKEY_RST_EN_MASK << PWRKEY_RST_EN_SHIFT,
+				   PWRKEY_RST_EN_MASK << PWRKEY_RST_EN_SHIFT);
+		regmap_update_bits(keys->regmap, pmic_rst_reg,
+				   HOMEKEY_RST_EN_MASK << HOMEKEY_RST_EN_SHIFT,
+				   0);
+		break;
+	case LP_TWOKEY:
+		regmap_update_bits(keys->regmap, pmic_rst_reg,
+				   PWRKEY_RST_EN_MASK << PWRKEY_RST_EN_SHIFT,
+				   PWRKEY_RST_EN_MASK << PWRKEY_RST_EN_SHIFT);
+		regmap_update_bits(keys->regmap, pmic_rst_reg,
+				   HOMEKEY_RST_EN_MASK << HOMEKEY_RST_EN_SHIFT,
+				   HOMEKEY_RST_EN_MASK << HOMEKEY_RST_EN_SHIFT);
+		break;
+	case LP_DISABLE:
+		regmap_update_bits(keys->regmap, pmic_rst_reg,
+				   PWRKEY_RST_EN_MASK << PWRKEY_RST_EN_SHIFT,
+				   0);
+		regmap_update_bits(keys->regmap, pmic_rst_reg,
+				   HOMEKEY_RST_EN_MASK << HOMEKEY_RST_EN_SHIFT,
+				   0);
+		break;
+	default:
+		break;
+	}
+}
+
+static irqreturn_t mtk_pmic_keys_irq_handler_thread(int irq, void *data)
+{
+	struct pmic_keys_info *info = data;
+	u32 key_deb, pressed;
+
+	regmap_read(info->keys->regmap, info->regs->deb_reg, &key_deb);
+
+	key_deb &= info->regs->deb_mask;
+
+	pressed = !key_deb;
+
+	input_report_key(info->keys->input_dev, info->keycode, pressed);
+	input_sync(info->keys->input_dev);
+
+	dev_dbg(info->keys->dev, "(%s) key =%d using PMIC\n",
+		 pressed ? "pressed" : "released", info->keycode);
+
+	return IRQ_HANDLED;
+}
+
+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);
+
+	input_set_capability(keys->input_dev, EV_KEY, info->keycode);
+
+	return 0;
+}
+
+static const struct of_device_id of_pmic_keys_match_tbl[] = {
+	{
+		.compatible = "mediatek,mt6397-keys",
+		.data = &mt6397_regs,
+	}, {
+		.compatible = "mediatek,mt6323-keys",
+		.data = &mt6323_regs,
+	}, {
+		/* sentinel */
+	}
+};
+MODULE_DEVICE_TABLE(of, of_pmic_keys_match_tbl);
+
+static int mtk_pmic_keys_probe(struct platform_device *pdev)
+{
+	int error, index = 0;
+	unsigned int keycount;
+	unsigned int keycodes[PMIC_MAX_KEY_COUNT];
+	unsigned int wakeup_srcs[PMIC_MAX_KEY_COUNT];
+	struct mt6397_chip *pmic_chip = dev_get_drvdata(pdev->dev.parent);
+	struct mtk_pmic_keys *keys;
+	const struct pmic_regs *pmic_regs;
+	struct input_dev *input_dev;
+	const struct of_device_id *of_id =
+		of_match_device(of_pmic_keys_match_tbl, &pdev->dev);
+
+	keys = devm_kzalloc(&pdev->dev, sizeof(*keys), GFP_KERNEL);
+	if (!keys)
+		return -ENOMEM;
+
+	keys->dev = &pdev->dev;
+	keys->regmap = pmic_chip->regmap;
+	pmic_regs = of_id->data;
+
+	keys->input_dev = input_dev = devm_input_allocate_device(keys->dev);
+	if (!input_dev) {
+		dev_err(keys->dev, "input allocate device fail.\n");
+		return -ENOMEM;
+	}
+
+	input_dev->name = "mtk-pmic-keys";
+	input_dev->id.bustype = BUS_HOST;
+	input_dev->id.vendor = 0x0001;
+	input_dev->id.product = 0x0001;
+	input_dev->id.version = 0x0001;
+
+	keycount = device_property_read_u32_array(keys->dev, "linux,keycodes",
+						  NULL, 0);
+	if (keycount > PMIC_MAX_KEY_COUNT) {
+		dev_err(keys->dev, "too many keys defined (%d)\n", keycount);
+		return -EINVAL;
+	}
+
+	error = device_property_read_u32_array(keys->dev, "linux,keycodes",
+					       keycodes, keycount);
+	if (error) {
+		dev_err(keys->dev,
+			"failed to read linux,keycode property: %d\n", error);
+		return error;
+	}
+
+	error = device_property_read_u32_array(keys->dev, "wakeup-source",
+					       wakeup_srcs, keycount);
+	if (error) {
+		dev_err(keys->dev,
+			"failed to read wakeup-source property: %d\n", error);
+		return error;
+	}
+
+	for (index = 0; index < keycount; index++) {
+		keys->keys[index].regs = &pmic_regs->keys_regs[index];
+
+		keys->keys[index].irq = platform_get_irq(pdev, index);
+		if (keys->keys[index].irq < 0)
+			return keys->keys[index].irq;
+
+		keys->keys[index].keycode = keycodes[index];
+
+		if (wakeup_srcs[index])
+			keys->keys[index].wakeup = true;
+
+		error = mtk_pmic_key_setup(keys, &keys->keys[index]);
+		if (error)
+			return error;
+	}
+
+	error = input_register_device(input_dev);
+	if (error) {
+		dev_err(&pdev->dev,
+			"register input device failed (%d)\n", error);
+		return error;
+	}
+
+	long_press_reset_setup(keys, pmic_regs->pmic_rst_reg);
+
+	return 0;
+}
+
+static struct platform_driver pmic_keys_pdrv = {
+	.probe = mtk_pmic_keys_probe,
+	.driver = {
+		   .name = "mtk-pmic-keys",
+		   .of_match_table = of_pmic_keys_match_tbl,
+	},
+};
+
+module_platform_driver(pmic_keys_pdrv);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Chen Zhong <chen.zhong@mediatek.com>");
+MODULE_DESCRIPTION("MTK pmic-keys driver v0.1");
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v3 1/5] mfd: mt6397: create irq mappings in mfd core driver
From: Chen Zhong @ 2017-08-25  6:32 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, Chen Zhong, Linus Walleij, Jaechul Lee,
	linux-input, devicetree, linux-arm-kernel, linux-mediatek,
	linux-kernel, linux-rtc
In-Reply-To: <1503642753-12385-1-git-send-email-chen.zhong@mediatek.com>

The core driver should create and manage irq mappings instead of
leaf drivers. This patch change to pass irq domain to
devm_mfd_add_devices() and it will create mapping for irq resources
automatically. And remove irq mapping in rtc driver since this has
been done in core driver.

Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Chen Zhong <chen.zhong@mediatek.com>
---
 drivers/mfd/mt6397-core.c |    4 ++--
 drivers/rtc/rtc-mt6397.c  |    7 +++----
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c
index 04a601f..6546d7f 100644
--- a/drivers/mfd/mt6397-core.c
+++ b/drivers/mfd/mt6397-core.c
@@ -289,7 +289,7 @@ static int mt6397_probe(struct platform_device *pdev)
 
 		ret = devm_mfd_add_devices(&pdev->dev, -1, mt6323_devs,
 					   ARRAY_SIZE(mt6323_devs), NULL,
-					   0, NULL);
+					   0, pmic->irq_domain);
 		break;
 
 	case MT6397_CID_CODE:
@@ -304,7 +304,7 @@ static int mt6397_probe(struct platform_device *pdev)
 
 		ret = devm_mfd_add_devices(&pdev->dev, -1, mt6397_devs,
 					   ARRAY_SIZE(mt6397_devs), NULL,
-					   0, NULL);
+					   0, pmic->irq_domain);
 		break;
 
 	default:
diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
index 1a61fa5..385f830 100644
--- a/drivers/rtc/rtc-mt6397.c
+++ b/drivers/rtc/rtc-mt6397.c
@@ -322,10 +322,9 @@ static int mtk_rtc_probe(struct platform_device *pdev)
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	rtc->addr_base = res->start;
 
-	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	rtc->irq = irq_create_mapping(mt6397_chip->irq_domain, res->start);
-	if (rtc->irq <= 0)
-		return -EINVAL;
+	rtc->irq = platform_get_irq(pdev, 0);
+	if (rtc->irq < 0)
+		return rtc->irq;
 
 	rtc->regmap = mt6397_chip->regmap;
 	rtc->dev = &pdev->dev;
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v3 0/5] Add MediaTek PMIC keys support
From: Chen Zhong @ 2017-08-25  6:32 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, Chen Zhong, Linus Walleij, Jaechul Lee,
	linux-input, devicetree, linux-arm-kernel, linux-mediatek,
	linux-kernel, linux-rtc

MediaTek PMIC are multi-function devices that can handle key interrupts,
typically there are two keys attached to PMIC, which called pwrkey
and homekey. PWRKEY usually used to wake up system from sleep. Homekey
can used as volume down key due to board design. Long press keys can
shutdown PMIC, the mode can be choose to be one key only or two keys
together.
This series add support for key functions for MediaTek PMIC MT6397/MT6323.

Chen Zhong (5):
  mfd: mt6397: create irq mappings in mfd core driver
  dt-bindings: input: Add document bindings for mtk-pmic-keys
  dt-bindings: mfd: Add bindings for the keys as subnode of PMIC
  input: Add MediaTek PMIC keys support
  mfd: mt6397: Add PMIC keys support to MT6397 driver

 .../devicetree/bindings/input/mtk-pmic-keys.txt    |  38 +++
 Documentation/devicetree/bindings/mfd/mt6397.txt   |   6 +
 drivers/input/keyboard/Kconfig                     |   9 +
 drivers/input/keyboard/Makefile                    |   1 +
 drivers/input/keyboard/mtk-pmic-keys.c             | 308 +++++++++++++++++++++
 drivers/mfd/mt6397-core.c                          |  26 +-
 drivers/rtc/rtc-mt6397.c                           |   7 +-
 7 files changed, 388 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/mtk-pmic-keys.txt
 create mode 100644 drivers/input/keyboard/mtk-pmic-keys.c

-- 
1.9.1

^ permalink raw reply

* Re: rtc: ds1307: call the platform's logic for handle IRQs.
From: Enric Balletbo i Serra @ 2017-08-25  6:43 UTC (permalink / raw)
  To: Heiner Kallweit, Alexandre Belloni; +Cc: linux-rtc
In-Reply-To: <f5cf7417-ac7a-4d52-56e3-c89c10f74df0@gmail.com>

Hi Heiner,

On 24/08/17 22:53, Heiner Kallweit wrote:
> Am 24.08.2017 um 22:44 schrieb Heiner Kallweit:
>> Am 24.08.2017 um 22:13 schrieb Enric Balletbo i Serra:
>>> Hi Heiner
>>>
>>> On 24/08/17 21:16, Heiner Kallweit wrote:
>>>> Am 24.08.2017 um 20:46 schrieb Heiner Kallweit:
>>>>> Hi Enric,
>>>>>
>>>>> I just saw your submitted patch on patchwork. As I haven't been subscribed
>>>>> to linux-rtc list yet, I can't reply to the original mail.
>>>>>
>>>>> Few remarks:
>>>>> I think the same can be achieved easier (apart from the fact that member
>>>>> irq was just removed from struct ds1307).
>>>>> The curent call to device_set_wakeup_capable has to be replaced with
>>>>> device_init_wakeup, in addition we have to call dev_pm_set_wake_irq to
>>>>> register the interrupt with the Linux wakeup core. Then the core takes
>>>>> care of everything.
>>>>>
>>>
>>> Oh I just saw your patches, thanks to advice me about them.
>>>
>>>> After further checking not even this may be necessary.
>>>> If flag I2C_CLIENT_WAKE is set for the i2c client then i2c_device_probe()
>>>> enables the device as wakeup device and registers the interrupt with the
>>>> wakeup core.
>>>> If the i2c client is defined via DT, then of_i2c_register_device() sets
>>>> this flag based on the "wakeup-source" property.
>>>> Is your device configured via DT? If yes, did you check whether wakeup
>>>> works w/o your patch with just setting property wakeup-source ?
>>>>
>>>
>>> Yes, device is configured via DT and without my patch, just setting property
>>> wakeup-source didn't work, the reason is that code:
>>>
>>> http://elixir.free-electrons.com/linux/v4.13-rc6/source/drivers/rtc/rtc-ds1307.c#L1382
>>>
>> This shouldn't be the reason because registering the interrupt with the wakeup core
>> happens earlier in i2c_device_probe().
>> Can you check whether dev_pm_set_wake_irq() is actually called with the right
>> irq number in i2c_device_probe()?
>>

You have reason again, I did something wrong on my tests and read the code in a
wrong way, to sum up, adding both, the interrupt pin and the wakeup-source
propriety, works as expected. So forget this patch. Sorry for the noise and
thanks for replying and look at this.

> See also description of commit 4990d4fe327b9d9a7a
> "PM / Wakeirq: Add automated device wake IRQ handling" from 2015.
> It explains why the code you want to add shouldn't be needed any longer.
> 
>> Would be interesting to see the DT config of your rtc.


        rtc0: rtc@68 {
                compatible = "dallas,ds1339";
                pinctrl-names = "default";
                pinctrl-0 = <&rtc0_irq_pins>;
                interrupt-parent = <&gpio0>;
                interrupts = <23 IRQ_TYPE_EDGE_FALLING>; /* gpio 23 */
                wakeup-source;
                trickle-resistor-ohms = <2000>;
                reg = <0x68>;
        };

>>
>>> I didn't test on top of your patches so let me rebase/rethink my patch on top of
>>> yours.
>>>
>>>>
>>>>> See also rtc-ds1343, although I think the calls to enable/disable_irq_wake
>>>>> are not needed there because the core takes care of this already
>>>>> (enable_irq_wake is called from dev_pm_arm_wake_irq).
>>>>>
>>>>> Rgds, Heiner
>>>>>
>>>>
>>>
>>> Thanks,
>>>  Enric
>>>
>>
> 

^ permalink raw reply

* [PATCH RESEND] rtc: sun6i: Add support for the external oscillator gate
From: Maxime Ripard @ 2017-08-25  7:42 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Chen-Yu Tsai, Maxime Ripard, linux-rtc, linux-arm-kernel,
	linux-kernel

The RTC can output its 32kHz clock outside of the SoC, for example to clock
a WiFi chip.

Create a new clock that other devices will be able to retrieve, while
maintaining the DT stability by providing a default name for that clock if
clock-output-names doesn't list one.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 .../devicetree/bindings/rtc/sun6i-rtc.txt          |  4 ++--
 drivers/rtc/rtc-sun6i.c                            | 24 +++++++++++++++++++---
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/rtc/sun6i-rtc.txt b/Documentation/devicetree/bindings/rtc/sun6i-rtc.txt
index 945934918b71..d5e26d313f62 100644
--- a/Documentation/devicetree/bindings/rtc/sun6i-rtc.txt
+++ b/Documentation/devicetree/bindings/rtc/sun6i-rtc.txt
@@ -10,7 +10,7 @@ Required properties:
 
 Required properties for new device trees
 - clocks	: phandle to the 32kHz external oscillator
-- clock-output-names : name of the LOSC clock created
+- clock-output-names : names of the LOSC and its external output clocks created
 - #clock-cells  : must be equals to 1. The RTC provides two clocks: the
 		  LOSC and its external output, with index 0 and 1
 		  respectively.
@@ -21,7 +21,7 @@ rtc: rtc@01f00000 {
 	compatible = "allwinner,sun6i-a31-rtc";
 	reg = <0x01f00000 0x54>;
 	interrupts = <0 40 4>, <0 41 4>;
-	clock-output-names = "osc32k";
+	clock-output-names = "osc32k", "osc32k-out";
 	clocks = <&ext_osc32k>;
 	#clock-cells = <1>;
 };
diff --git a/drivers/rtc/rtc-sun6i.c b/drivers/rtc/rtc-sun6i.c
index 305c4d043f61..3d2216ccd860 100644
--- a/drivers/rtc/rtc-sun6i.c
+++ b/drivers/rtc/rtc-sun6i.c
@@ -73,6 +73,9 @@
 #define SUN6I_ALARM_CONFIG			0x0050
 #define SUN6I_ALARM_CONFIG_WAKEUP		BIT(0)
 
+#define SUN6I_LOSC_OUT_GATING			0x0060
+#define SUN6I_LOSC_OUT_GATING_EN		BIT(0)
+
 /*
  * Get date values
  */
@@ -125,6 +128,7 @@ struct sun6i_rtc_dev {
 	struct clk_hw hw;
 	struct clk_hw *int_osc;
 	struct clk *losc;
+	struct clk *ext_losc;
 
 	spinlock_t lock;
 };
@@ -188,13 +192,14 @@ static void __init sun6i_rtc_clk_init(struct device_node *node)
 	struct clk_init_data init = {
 		.ops		= &sun6i_rtc_osc_ops,
 	};
+	const char *clkout_name = "osc32k-out";
 	const char *parents[2];
 
 	rtc = kzalloc(sizeof(*rtc), GFP_KERNEL);
 	if (!rtc)
 		return;
 
-	clk_data = kzalloc(sizeof(*clk_data) + sizeof(*clk_data->hws),
+	clk_data = kzalloc(sizeof(*clk_data) + (sizeof(*clk_data->hws) * 2),
 			   GFP_KERNEL);
 	if (!clk_data)
 		return;
@@ -235,7 +240,8 @@ static void __init sun6i_rtc_clk_init(struct device_node *node)
 
 	init.parent_names = parents;
 	init.num_parents = of_clk_get_parent_count(node) + 1;
-	of_property_read_string(node, "clock-output-names", &init.name);
+	of_property_read_string_index(node, "clock-output-names", 0,
+				      &init.name);
 
 	rtc->losc = clk_register(NULL, &rtc->hw);
 	if (IS_ERR(rtc->losc)) {
@@ -243,8 +249,20 @@ static void __init sun6i_rtc_clk_init(struct device_node *node)
 		return;
 	}
 
-	clk_data->num = 1;
+	of_property_read_string_index(node, "clock-output-names", 1,
+				      &clkout_name);
+	rtc->ext_losc = clk_register_gate(NULL, clkout_name, rtc->hw.init->name,
+					  0, rtc->base + SUN6I_LOSC_OUT_GATING,
+					  SUN6I_LOSC_OUT_GATING_EN, 0,
+					  &rtc->lock);
+	if (IS_ERR(rtc->ext_losc)) {
+		pr_crit("Couldn't register the LOSC external gate\n");
+		return;
+	}
+
+	clk_data->num = 2;
 	clk_data->hws[0] = &rtc->hw;
+	clk_data->hws[1] = __clk_get_hw(rtc->ext_losc);
 	of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
 	return;
 
-- 
2.13.5

^ permalink raw reply related

* Re: [PATCH RESEND] rtc: sun6i: Add support for the external oscillator gate
From: Alexandre Belloni @ 2017-08-25  8:33 UTC (permalink / raw)
  To: Maxime Ripard; +Cc: Chen-Yu Tsai, linux-rtc, linux-arm-kernel, linux-kernel
In-Reply-To: <20170825074202.18743-1-maxime.ripard@free-electrons.com>

On 25/08/2017 at 09:42:02 +0200, Maxime Ripard wrote:
> The RTC can output its 32kHz clock outside of the SoC, for example to clock
> a WiFi chip.
> 
> Create a new clock that other devices will be able to retrieve, while
> maintaining the DT stability by providing a default name for that clock if
> clock-output-names doesn't list one.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  .../devicetree/bindings/rtc/sun6i-rtc.txt          |  4 ++--
>  drivers/rtc/rtc-sun6i.c                            | 24 +++++++++++++++++++---
>  2 files changed, 23 insertions(+), 5 deletions(-)
> 
Applied, thanks.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* Re: [PATCH v4 2/8] rtc: goldfish: Add RTC driver for Android emulator
From: Alexandre Belloni @ 2017-08-25  8:34 UTC (permalink / raw)
  To: Aleksandar Markovic
  Cc: linux-mips, Miodrag Dinic, Goran Ferenc, Aleksandar Markovic,
	Alessandro Zummo, Bo Hu, David S. Miller, Douglas Leung,
	Greg Kroah-Hartman, James Hogan, Jin Qian, linux-kernel,
	linux-rtc, Mauro Carvalho Chehab, Paul Burton, Petar Jovanovic,
	Raghu Gandham, Randy Dunlap
In-Reply-To: <1503061833-26563-3-git-send-email-aleksandar.markovic@rt-rk.com>

On 18/08/2017 at 15:08:54 +0200, Aleksandar Markovic wrote:
> From: Miodrag Dinic <miodrag.dinic@imgtec.com>
> 
> Add device driver for a virtual RTC device in Android emulator.
> 
> The compatible string used by OS for binding the driver is defined
> as "google,goldfish-rtc".
> 
> Signed-off-by: Miodrag Dinic <miodrag.dinic@imgtec.com>
> Signed-off-by: Goran Ferenc <goran.ferenc@imgtec.com>
> Signed-off-by: Aleksandar Markovic <aleksandar.markovic@imgtec.com>
> ---
>  MAINTAINERS                |   1 +
>  drivers/rtc/Kconfig        |   8 ++
>  drivers/rtc/Makefile       |   1 +
>  drivers/rtc/rtc-goldfish.c | 237 +++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 247 insertions(+)
>  create mode 100644 drivers/rtc/rtc-goldfish.c
> 
Applied, thanks.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* Re: [PATCH v4 1/8] Documentation: Add device tree binding for Goldfish RTC driver
From: Alexandre Belloni @ 2017-08-25  8:34 UTC (permalink / raw)
  To: Aleksandar Markovic
  Cc: linux-mips, Aleksandar Markovic, Miodrag Dinic, Goran Ferenc,
	Alessandro Zummo, Bo Hu, David S. Miller, devicetree,
	Douglas Leung, Greg Kroah-Hartman, James Hogan, Jin Qian,
	linux-kernel, linux-rtc, Mark Rutland, Mauro Carvalho Chehab,
	Paul Burton, Petar Jovanovic, Raghu Gandham, Randy Dunlap,
	Rob Herring
In-Reply-To: <1503061833-26563-2-git-send-email-aleksandar.markovic@rt-rk.com>

On 18/08/2017 at 15:08:53 +0200, Aleksandar Markovic wrote:
> From: Aleksandar Markovic <aleksandar.markovic@imgtec.com>
> 
> Add documentation for DT binding of Goldfish RTC driver. The compatible
> string used by OS for binding the driver is "google,goldfish-rtc".
> 
> Signed-off-by: Miodrag Dinic <miodrag.dinic@imgtec.com>
> Signed-off-by: Goran Ferenc <goran.ferenc@imgtec.com>
> Signed-off-by: Aleksandar Markovic <aleksandar.markovic@imgtec.com>
> ---
>  .../devicetree/bindings/rtc/google,goldfish-rtc.txt     | 17 +++++++++++++++++
>  MAINTAINERS                                             |  5 +++++
>  2 files changed, 22 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/rtc/google,goldfish-rtc.txt
> 
Applied, thanks.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* Re: [PATCH v3 1/5] mfd: mt6397: create irq mappings in mfd core driver
From: Alexandre Belloni @ 2017-08-25  8:39 UTC (permalink / raw)
  To: Chen Zhong
  Cc: Dmitry Torokhov, Rob Herring, Mark Rutland, Matthias Brugger,
	Lee Jones, Eddie Huang, Alessandro Zummo, 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-2-git-send-email-chen.zhong@mediatek.com>

On 25/08/2017 at 14:32:29 +0800, Chen Zhong wrote:
> The core driver should create and manage irq mappings instead of
> leaf drivers. This patch change to pass irq domain to
> devm_mfd_add_devices() and it will create mapping for irq resources
> automatically. And remove irq mapping in rtc driver since this has
> been done in core driver.
> 
> Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Chen Zhong <chen.zhong@mediatek.com>
Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>

> ---
>  drivers/mfd/mt6397-core.c |    4 ++--
>  drivers/rtc/rtc-mt6397.c  |    7 +++----
>  2 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mfd/mt6397-core.c b/drivers/mfd/mt6397-core.c
> index 04a601f..6546d7f 100644
> --- a/drivers/mfd/mt6397-core.c
> +++ b/drivers/mfd/mt6397-core.c
> @@ -289,7 +289,7 @@ static int mt6397_probe(struct platform_device *pdev)
>  
>  		ret = devm_mfd_add_devices(&pdev->dev, -1, mt6323_devs,
>  					   ARRAY_SIZE(mt6323_devs), NULL,
> -					   0, NULL);
> +					   0, pmic->irq_domain);
>  		break;
>  
>  	case MT6397_CID_CODE:
> @@ -304,7 +304,7 @@ static int mt6397_probe(struct platform_device *pdev)
>  
>  		ret = devm_mfd_add_devices(&pdev->dev, -1, mt6397_devs,
>  					   ARRAY_SIZE(mt6397_devs), NULL,
> -					   0, NULL);
> +					   0, pmic->irq_domain);
>  		break;
>  
>  	default:
> diff --git a/drivers/rtc/rtc-mt6397.c b/drivers/rtc/rtc-mt6397.c
> index 1a61fa5..385f830 100644
> --- a/drivers/rtc/rtc-mt6397.c
> +++ b/drivers/rtc/rtc-mt6397.c
> @@ -322,10 +322,9 @@ static int mtk_rtc_probe(struct platform_device *pdev)
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	rtc->addr_base = res->start;
>  
> -	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> -	rtc->irq = irq_create_mapping(mt6397_chip->irq_domain, res->start);
> -	if (rtc->irq <= 0)
> -		return -EINVAL;
> +	rtc->irq = platform_get_irq(pdev, 0);
> +	if (rtc->irq < 0)
> +		return rtc->irq;
>  
>  	rtc->regmap = mt6397_chip->regmap;
>  	rtc->dev = &pdev->dev;
> -- 
> 1.7.9.5
> 

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* [PATCH 0/5] rtc: ds1307: factor out more stuff from ds1307_probe and improve ds1307_set_time
From: Heiner Kallweit @ 2017-08-25 19:30 UTC (permalink / raw)
  To: Alexandre Belloni; +Cc: linux-rtc

Final goal of the refactoring is to abstract everything that the chips
have in common and handle it in generic code. Then we can get rid of
all the "switch (chip)" and "if (chip == xxx)" code.

To give one example:
A lot of chips have a bit for setting 12hr / 24hr mode. However some
chips have this config bit in the timekeeping registers, others in
a config register, and on some chips it's inverted.
But the functionality of the bit is always the same.

Ultimately adding support for a chip just requires to add one config
structure member.

The way to reach this goal is a long one and to faciliate reviewing
the patches I'll split them into series of 5 to 10 patches.

Heiner Kallweit (5):
  rtc: ds1307: factor out determining the chip type
  rtc: ds1307: factor out trickle charger initialization
  rtc: ds1307: factor out fixing the weekday
  rtc: ds1307: introduce constants for the timekeeping register masks
  rtc: ds1307: improve ds1307_set_time to respect config flag bits

 drivers/rtc/rtc-ds1307.c | 256 ++++++++++++++++++++++++++---------------------
 1 file changed, 140 insertions(+), 116 deletions(-)

-- 
2.14.1

^ permalink raw reply

* [PATCH 1/5] rtc: ds1307: factor out determining the chip type
From: Heiner Kallweit @ 2017-08-25 20:06 UTC (permalink / raw)
  To: Alexandre Belloni; +Cc: linux-rtc
In-Reply-To: <cb381496-4d55-996d-e2ae-18880330e386@gmail.com>

ds1307_probe is very long and confusing, so let's factor out
functionalities. As a first step factor out determining the chip type.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/rtc/rtc-ds1307.c | 37 ++++++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 4f00cea2..2b5c6d60 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -1329,6 +1329,21 @@ static void ds1307_clks_register(struct ds1307 *ds1307)
 
 #endif /* CONFIG_COMMON_CLK */
 
+static enum ds_type ds1307_get_type(struct device *dev,
+				    const struct i2c_device_id *id)
+{
+	const struct acpi_device_id *acpi_id;
+
+	if (dev->of_node)
+		return (enum ds_type) of_device_get_match_data(dev);
+
+	if (id)
+		return id->driver_data;
+
+	acpi_id = acpi_match_device(ACPI_PTR(ds1307_acpi_ids), dev);
+	return acpi_id ? acpi_id->driver_data : -ENODEV;
+}
+
 static const struct regmap_config regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
@@ -1365,23 +1380,11 @@ static int ds1307_probe(struct i2c_client *client,
 
 	i2c_set_clientdata(client, ds1307);
 
-	if (client->dev.of_node) {
-		ds1307->type = (enum ds_type)
-			of_device_get_match_data(&client->dev);
-		chip = &chips[ds1307->type];
-	} else if (id) {
-		chip = &chips[id->driver_data];
-		ds1307->type = id->driver_data;
-	} else {
-		const struct acpi_device_id *acpi_id;
-
-		acpi_id = acpi_match_device(ACPI_PTR(ds1307_acpi_ids),
-					    ds1307->dev);
-		if (!acpi_id)
-			return -ENODEV;
-		chip = &chips[acpi_id->driver_data];
-		ds1307->type = acpi_id->driver_data;
-	}
+	ds1307->type = ds1307_get_type(&client->dev, id);
+	if (ds1307->type < 0)
+		return ds1307->type;
+
+	chip = &chips[ds1307->type];
 
 	want_irq = client->irq > 0 && chip->alarm;
 
-- 
2.14.1

^ permalink raw reply related

* [PATCH 2/5] rtc: ds1307: factor out trickle charger initialization
From: Heiner Kallweit @ 2017-08-25 20:06 UTC (permalink / raw)
  To: Alexandre Belloni; +Cc: linux-rtc
In-Reply-To: <cb381496-4d55-996d-e2ae-18880330e386@gmail.com>

Rename the existing function ds1307_trickle_init to ds1307_trickle_setup
and factor out trickle charger initialization from ds1307_probe into a
new function ds1307_trickle_init.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/rtc/rtc-ds1307.c | 48 ++++++++++++++++++++++++++++--------------------
 1 file changed, 28 insertions(+), 20 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 2b5c6d60..ea88e4b3 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -970,8 +970,8 @@ static u8 do_trickle_setup_ds1339(struct ds1307 *ds1307,
 	return setup;
 }
 
-static u8 ds1307_trickle_init(struct ds1307 *ds1307,
-			      const struct chip_desc *chip)
+static u8 ds1307_trickle_setup(struct ds1307 *ds1307,
+			       const struct chip_desc *chip)
 {
 	uint32_t ohms;
 	bool diode = true;
@@ -1344,6 +1344,30 @@ static enum ds_type ds1307_get_type(struct device *dev,
 	return acpi_id ? acpi_id->driver_data : -ENODEV;
 }
 
+static void ds1307_trickle_init(struct ds1307 *ds1307)
+{
+	struct ds1307_platform_data *pdata = dev_get_platdata(ds1307->dev);
+	const struct chip_desc *chip = &chips[ds1307->type];
+	u8 trickle_charger_setup;
+
+	if (!chip->trickle_charger_reg)
+		return;
+
+	if (pdata)
+		trickle_charger_setup = pdata->trickle_charger_setup;
+	else
+		trickle_charger_setup = ds1307_trickle_setup(ds1307, chip);
+
+	if (trickle_charger_setup) {
+		trickle_charger_setup |= DS13XX_TRICKLE_CHARGER_MAGIC;
+		dev_dbg(ds1307->dev,
+			"writing trickle charger info 0x%x to 0x%x\n",
+			trickle_charger_setup, chip->trickle_charger_reg);
+		regmap_write(ds1307->regmap, chip->trickle_charger_reg,
+			     trickle_charger_setup);
+	}
+}
+
 static const struct regmap_config regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
@@ -1359,10 +1383,8 @@ static int ds1307_probe(struct i2c_client *client,
 	bool			want_irq;
 	bool			ds1307_can_wakeup_device = false;
 	unsigned char		*buf;
-	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);
 	if (!ds1307)
@@ -1385,25 +1407,11 @@ static int ds1307_probe(struct i2c_client *client,
 		return ds1307->type;
 
 	chip = &chips[ds1307->type];
-
 	want_irq = client->irq > 0 && chip->alarm;
-
-	if (!pdata)
-		trickle_charger_setup = ds1307_trickle_init(ds1307, chip);
-	else if (pdata->trickle_charger_setup)
-		trickle_charger_setup = pdata->trickle_charger_setup;
-
-	if (trickle_charger_setup && chip->trickle_charger_reg) {
-		trickle_charger_setup |= DS13XX_TRICKLE_CHARGER_MAGIC;
-		dev_dbg(ds1307->dev,
-			"writing trickle charger info 0x%x to 0x%x\n",
-			trickle_charger_setup, chip->trickle_charger_reg);
-		regmap_write(ds1307->regmap, chip->trickle_charger_reg,
-			     trickle_charger_setup);
-	}
-
 	buf = ds1307->regs;
 
+	ds1307_trickle_init(ds1307);
+
 #ifdef CONFIG_OF
 /*
  * For devices with no IRQ directly connected to the SoC, the RTC chip
-- 
2.14.1

^ permalink raw reply related

* [PATCH 3/5] rtc: ds1307: factor out fixing the weekday
From: Heiner Kallweit @ 2017-08-25 20:06 UTC (permalink / raw)
  To: Alexandre Belloni; +Cc: linux-rtc
In-Reply-To: <cb381496-4d55-996d-e2ae-18880330e386@gmail.com>

Factor out checking and fixing the weekday.

In addition fix two issues with the old implementation:
- For variable timestamp use correct type time64_t instead of
  unsigned long which may be just 32bit long.
- Updating the weekday register only may be racy, therefore write all
  timekeeping registers.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/rtc/rtc-ds1307.c | 42 +++++++++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index ea88e4b3..69f514b6 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -1368,6 +1368,29 @@ static void ds1307_trickle_init(struct ds1307 *ds1307)
 	}
 }
 
+static void ds1307_fix_weekday(struct ds1307 *ds1307)
+{
+	struct rtc_time tm;
+	time64_t timestamp;
+	int wday;
+
+	if (ds1307_get_time(ds1307->dev, &tm))
+		return;
+
+	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
+	 * Set not only wday but complete date to avoid potential races.
+	 */
+	if (wday != tm.tm_wday)
+		ds1307_set_time(ds1307->dev, &tm);
+}
+
 static const struct regmap_config regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
@@ -1378,13 +1401,11 @@ 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		*buf;
-	struct rtc_time		tm;
-	unsigned long		timestamp;
 
 	ds1307 = devm_kzalloc(&client->dev, sizeof(struct ds1307), GFP_KERNEL);
 	if (!ds1307)
@@ -1642,20 +1663,7 @@ static int ds1307_probe(struct i2c_client *client,
 	 * 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);
+	ds1307_fix_weekday(ds1307);
 
 	if (want_irq || ds1307_can_wakeup_device) {
 		device_set_wakeup_capable(ds1307->dev, true);
-- 
2.14.1

^ permalink raw reply related

* [PATCH 4/5] rtc: ds1307: introduce constants for the timekeeping register masks
From: Heiner Kallweit @ 2017-08-25 20:06 UTC (permalink / raw)
  To: Alexandre Belloni; +Cc: linux-rtc
In-Reply-To: <cb381496-4d55-996d-e2ae-18880330e386@gmail.com>

So far for masking the relevant bits of the timekeeping registers
magic numbers are used. Replace them with proper constants.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/rtc/rtc-ds1307.c | 63 ++++++++++++++++++++++++++----------------------
 1 file changed, 34 insertions(+), 29 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 69f514b6..a43a80b2 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -54,22 +54,29 @@ enum ds_type {
 
 /* RTC registers don't differ much, except for the century flag */
 #define DS1307_REG_SECS		0x00	/* 00-59 */
+#define DS1307_SECS_MASK	GENMASK(6, 0)
 #	define DS1307_BIT_CH		0x80
 #	define DS1340_BIT_nEOSC		0x80
 #	define MCP794XX_BIT_ST		0x80
 #define DS1307_REG_MIN		0x01	/* 00-59 */
+#define DS1307_MIN_MASK		GENMASK(6, 0)
 #	define M41T0_BIT_OF		0x80
 #define DS1307_REG_HOUR		0x02	/* 00-23, or 1-12{am,pm} */
+#define DS1307_HOUR_MASK	GENMASK(5, 0)
 #	define DS1307_BIT_12HR		0x40	/* in REG_HOUR */
 #	define DS1307_BIT_PM		0x20	/* in REG_HOUR */
 #	define DS1340_BIT_CENTURY_EN	0x80	/* in REG_HOUR */
 #	define DS1340_BIT_CENTURY	0x40	/* in REG_HOUR */
 #define DS1307_REG_WDAY		0x03	/* 01-07 */
+#define DS1307_WDAY_MASK	GENMASK(2, 0)
 #	define MCP794XX_BIT_VBATEN	0x08
 #define DS1307_REG_MDAY		0x04	/* 01-31 */
+#define DS1307_MDAY_MASK	GENMASK(5, 0)
 #define DS1307_REG_MONTH	0x05	/* 01-12 */
+#define DS1307_MONTH_MASK	GENMASK(4, 0)
 #	define DS1337_BIT_CENTURY	0x80	/* in REG_MONTH */
 #define DS1307_REG_YEAR		0x06	/* 00-99 */
+#define DS1307_YEAR_MASK	GENMASK(7, 0)
 
 /*
  * Other registers (control, status, alarms, trickle charge, NVRAM, etc)
@@ -395,36 +402,34 @@ static irqreturn_t ds1307_irq(int irq, void *dev_id)
 static int ds1307_get_time(struct device *dev, struct rtc_time *t)
 {
 	struct ds1307	*ds1307 = dev_get_drvdata(dev);
-	int		tmp, ret;
 	const struct chip_desc *chip = &chips[ds1307->type];
+	u8		*buf = ds1307->regs;
+	int		ret;
 
 	/* read the RTC date and time registers all at once */
-	ret = regmap_bulk_read(ds1307->regmap, chip->offset, ds1307->regs, 7);
+	ret = regmap_bulk_read(ds1307->regmap, chip->offset, buf, 7);
 	if (ret) {
 		dev_err(dev, "%s error %d\n", "read", ret);
 		return ret;
 	}
 
-	dev_dbg(dev, "%s: %7ph\n", "read", ds1307->regs);
+	dev_dbg(dev, "%s: %7ph\n", "read", buf);
 
 	/* if oscillator fail bit is set, no data can be trusted */
-	if (ds1307->type == m41t0 &&
-	    ds1307->regs[DS1307_REG_MIN] & M41T0_BIT_OF) {
+	if (ds1307->type == m41t0 && buf[DS1307_REG_MIN] & M41T0_BIT_OF) {
 		dev_warn_once(dev, "oscillator failed, set time!\n");
 		return -EINVAL;
 	}
 
-	t->tm_sec = bcd2bin(ds1307->regs[DS1307_REG_SECS] & 0x7f);
-	t->tm_min = bcd2bin(ds1307->regs[DS1307_REG_MIN] & 0x7f);
-	tmp = ds1307->regs[DS1307_REG_HOUR] & 0x3f;
-	t->tm_hour = bcd2bin(tmp);
-	t->tm_wday = bcd2bin(ds1307->regs[DS1307_REG_WDAY] & 0x07) - 1;
-	t->tm_mday = bcd2bin(ds1307->regs[DS1307_REG_MDAY] & 0x3f);
-	tmp = ds1307->regs[DS1307_REG_MONTH] & 0x1f;
-	t->tm_mon = bcd2bin(tmp) - 1;
-	t->tm_year = bcd2bin(ds1307->regs[DS1307_REG_YEAR]) + 100;
-
-	if (ds1307->regs[chip->century_reg] & chip->century_bit &&
+	t->tm_sec = bcd2bin(buf[DS1307_REG_SECS] & DS1307_SECS_MASK);
+	t->tm_min = bcd2bin(buf[DS1307_REG_MIN] & DS1307_MIN_MASK);
+	t->tm_hour = bcd2bin(buf[DS1307_REG_HOUR] & DS1307_HOUR_MASK);
+	t->tm_wday = bcd2bin(buf[DS1307_REG_WDAY] & DS1307_WDAY_MASK) - 1;
+	t->tm_mday = bcd2bin(buf[DS1307_REG_MDAY] & DS1307_MDAY_MASK);
+	t->tm_mon = bcd2bin(buf[DS1307_REG_MONTH] & DS1307_MONTH_MASK) - 1;
+	t->tm_year = bcd2bin(buf[DS1307_REG_YEAR] & DS1307_YEAR_MASK) + 100;
+
+	if (buf[chip->century_reg] & chip->century_bit &&
 	    IS_ENABLED(CONFIG_RTC_DRV_DS1307_CENTURY))
 		t->tm_year += 100;
 
@@ -522,10 +527,10 @@ static int ds1337_read_alarm(struct device *dev, struct rtc_wkalrm *t)
 	 * report alarm time (ALARM1); assume 24 hour and day-of-month modes,
 	 * and that all four fields are checked matches
 	 */
-	t->time.tm_sec = bcd2bin(ds1307->regs[0] & 0x7f);
-	t->time.tm_min = bcd2bin(ds1307->regs[1] & 0x7f);
-	t->time.tm_hour = bcd2bin(ds1307->regs[2] & 0x3f);
-	t->time.tm_mday = bcd2bin(ds1307->regs[3] & 0x3f);
+	t->time.tm_sec = bcd2bin(ds1307->regs[0] & DS1307_SECS_MASK);
+	t->time.tm_min = bcd2bin(ds1307->regs[1] & DS1307_MIN_MASK);
+	t->time.tm_hour = bcd2bin(ds1307->regs[2] & DS1307_HOUR_MASK);
+	t->time.tm_mday = bcd2bin(ds1307->regs[3] & DS1307_MDAY_MASK);
 
 	/* ... and status */
 	t->enabled = !!(ds1307->regs[7] & DS1337_BIT_A1IE);
@@ -689,10 +694,10 @@ static int rx8130_read_alarm(struct device *dev, struct rtc_wkalrm *t)
 
 	/* Report alarm 0 time assuming 24-hour and day-of-month modes. */
 	t->time.tm_sec = -1;
-	t->time.tm_min = bcd2bin(ald[0] & 0x7f);
-	t->time.tm_hour = bcd2bin(ald[1] & 0x7f);
+	t->time.tm_min = bcd2bin(ald[0] & DS1307_MIN_MASK);
+	t->time.tm_hour = bcd2bin(ald[1] & DS1307_HOUR_MASK);
 	t->time.tm_wday = -1;
-	t->time.tm_mday = bcd2bin(ald[2] & 0x7f);
+	t->time.tm_mday = bcd2bin(ald[2] & DS1307_MDAY_MASK);
 	t->time.tm_mon = -1;
 	t->time.tm_year = -1;
 	t->time.tm_yday = -1;
@@ -844,12 +849,12 @@ static int mcp794xx_read_alarm(struct device *dev, struct rtc_wkalrm *t)
 	t->enabled = !!(regs[0] & MCP794XX_BIT_ALM0_EN);
 
 	/* Report alarm 0 time assuming 24-hour and day-of-month modes. */
-	t->time.tm_sec = bcd2bin(ds1307->regs[3] & 0x7f);
-	t->time.tm_min = bcd2bin(ds1307->regs[4] & 0x7f);
-	t->time.tm_hour = bcd2bin(ds1307->regs[5] & 0x3f);
-	t->time.tm_wday = bcd2bin(ds1307->regs[6] & 0x7) - 1;
-	t->time.tm_mday = bcd2bin(ds1307->regs[7] & 0x3f);
-	t->time.tm_mon = bcd2bin(ds1307->regs[8] & 0x1f) - 1;
+	t->time.tm_sec = bcd2bin(ds1307->regs[3] & DS1307_SECS_MASK);
+	t->time.tm_min = bcd2bin(ds1307->regs[4] & DS1307_MIN_MASK);
+	t->time.tm_hour = bcd2bin(ds1307->regs[5] & DS1307_HOUR_MASK);
+	t->time.tm_wday = bcd2bin(ds1307->regs[6] & DS1307_WDAY_MASK) - 1;
+	t->time.tm_mday = bcd2bin(ds1307->regs[7] & DS1307_MDAY_MASK);
+	t->time.tm_mon = bcd2bin(ds1307->regs[8] & DS1307_MONTH_MASK) - 1;
 	t->time.tm_year = -1;
 	t->time.tm_yday = -1;
 	t->time.tm_isdst = -1;
-- 
2.14.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox