linux-kernel - Re: [PATCH V2 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 3 Feb 2016 09:56:42 -0300
From:	Javier Martinez Canillas <javier@....samsung.com>
To:	Laxman Dewangan <ldewangan@...dia.com>, lee.jones@...aro.org,
	alexandre.belloni@...e-electrons.com, k.kozlowski@...sung.com
Cc:	cw00.choi@...sung.com, linux-kernel@...r.kernel.org,
	rtc-linux@...glegroups.com
Subject: Re: [PATCH V2 5/5] rtc: max77686: move initialisation of rtc regmap,
 irq chip locally

Hello Laxman,

On 02/03/2016 06:30 AM, Laxman Dewangan wrote:
> To make RTC block of MAX77686/MAX77802 as independent driver,
> move the registration of i2c device, regmap for register access
> and irq_chip for interrupt support inside the RTC driver.
> Removed the same initialisation from MFD driver.
>
> Having this change will allow to reuse this driver for different
> PMIC/devices from Maxim Semiconductor if they kept same RTC IP on
> different PMIC. Some of examples as PMIC MAX77620, MAX20024 where
> same RTC IP used and hence driver for these chips will use this
> driver only for RTC support.
>
> Suggested-by: Krzysztof Kozlowski <k.kozlowski@...sung.com>
> Signed-off-by: Laxman Dewangan <ldewangan@...dia.com>
> CC: Krzysztof Kozlowski <k.kozlowski@...sung.com>
> CC: Javier Martinez Canillas <javier@....samsung.com>
>

[snip]

>
> diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
> index ab1f2cd..10984c4 100644
> --- a/drivers/rtc/rtc-max77686.c
> +++ b/drivers/rtc/rtc-max77686.c
> @@ -12,6 +12,7 @@
>    *
>    */
>
> +#include <linux/i2c.h>
>   #include <linux/slab.h>
>   #include <linux/rtc.h>
>   #include <linux/delay.h>
> @@ -22,6 +23,9 @@
>   #include <linux/irqdomain.h>
>   #include <linux/regmap.h>
>
> +#define MAX77686_I2C_ADDR_RTC		(0x0C >> 1)
> +#define INVALID_I2C_ADDR		(-1)
> +

Maybe call it MAX77686_INVALID_I2C_ADDR for consistency?

The patch looks good modulo the issues pointed out by Krzysztof, so after
fixing these feel free to add:

Reviewed-by: Javier Martinez Canillas <javier@....samsung.com>

Since you are going to re-spin another version, I'll wait for that to test.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America