From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mailout3.w1.samsung.com (mailout3.w1.samsung.com. [210.118.77.13]) by gmr-mx.google.com with ESMTPS id r2si3469374pfr.0.2016.01.20.17.05.54 for (version=TLS1 cipher=AES128-SHA bits=128/128); Wed, 20 Jan 2016 17:05:55 -0800 (PST) Received: from eucpsbgm2.samsung.com (unknown [203.254.199.245]) by mailout3.w1.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTP id <0O1A00I6J31Q7D80@mailout3.w1.samsung.com> for rtc-linux@googlegroups.com; Thu, 21 Jan 2016 01:05:50 +0000 (GMT) Subject: [rtc-linux] Re: [PATCH 4/8] rtc: max77686: Add an indirection level to access RTC registers To: Javier Martinez Canillas , linux-kernel@vger.kernel.org References: <1453310088-29985-1-git-send-email-javier@osg.samsung.com> <1453310088-29985-5-git-send-email-javier@osg.samsung.com> Cc: Kukjin Kim , rtc-linux@googlegroups.com, Chanwoo Choi , Alexandre Belloni , Laxman Dewangan , linux-samsung-soc@vger.kernel.org From: Krzysztof Kozlowski Message-id: <56A02EEF.2060604@samsung.com> Date: Thu, 21 Jan 2016 10:05:51 +0900 MIME-version: 1.0 In-reply-to: <1453310088-29985-5-git-send-email-javier@osg.samsung.com> Content-type: text/plain; charset=UTF-8 Reply-To: rtc-linux@googlegroups.com List-ID: List-Post: , List-Help: , List-Archive: , List-Unsubscribe: , On 21.01.2016 02:14, Javier Martinez Canillas wrote: > The max77686 driver is generic enough that can be used for other > Maxim RTC IP blocks but these might not have the same registers > layout so instead of accessing the registers directly, add a map > to translate offsets to the real registers addresses for each IP. > > Signed-off-by: Javier Martinez Canillas > --- > > drivers/rtc/rtc-max77686.c | 75 +++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 65 insertions(+), 10 deletions(-) > > diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c > index 441d163dcbeb..7316e41820c7 100644 > --- a/drivers/rtc/rtc-max77686.c > +++ b/drivers/rtc/rtc-max77686.c > @@ -41,6 +41,8 @@ > #define ALARM_ENABLE_SHIFT 7 > #define ALARM_ENABLE_MASK (1 << ALARM_ENABLE_SHIFT) > > +#define REG_RTC_NONE 0xdeadbeef > + > enum { > RTC_SEC = 0, > RTC_MIN, > @@ -55,6 +57,7 @@ enum { > struct rtc_driver_data { > unsigned long delay; > int mask; > + const unsigned int *map; > }; > > struct max77686_rtc_info { > @@ -77,9 +80,53 @@ enum MAX77686_RTC_OP { > MAX77686_RTC_READ, > }; > > +/* These are not registers but just offsets that are mapped to addresses */ > +enum rtc_reg { enum max77686_rtc_reg_offset? > + REG_RTC_CONTROLM = 0, > + REG_RTC_CONTROL, > + REG_RTC_UPDATE0, > + REG_RTC_UPDATE1, > + REG_WTSR_SMPL_CNTL, > + REG_RTC_SEC, > + REG_RTC_MIN, > + REG_RTC_HOUR, > + REG_RTC_WEEKDAY, > + REG_RTC_MONTH, > + REG_RTC_YEAR, > + REG_RTC_DATE, > + REG_ALARM1_SEC, > + REG_ALARM1_MIN, > + REG_ALARM1_HOUR, > + REG_ALARM1_WEEKDAY, > + REG_ALARM1_MONTH, > + REG_ALARM1_YEAR, > + REG_ALARM1_DATE, > + REG_ALARM2_SEC, > + REG_ALARM2_MIN, > + REG_ALARM2_HOUR, > + REG_ALARM2_WEEKDAY, > + REG_ALARM2_MONTH, > + REG_ALARM2_YEAR, > + REG_ALARM2_DATE, > + REG_RTC_END, > +}; > + A short comment what is mapped into what would be appreciated. > +static const unsigned int max77686_map[REG_RTC_END] = { > + MAX77686_RTC_CONTROLM, MAX77686_RTC_CONTROL, MAX77686_RTC_UPDATE0, > + REG_RTC_NONE, MAX77686_WTSR_SMPL_CNTL, MAX77686_RTC_SEC, > + MAX77686_RTC_MIN, MAX77686_RTC_HOUR, MAX77686_RTC_WEEKDAY, > + MAX77686_RTC_MONTH, MAX77686_RTC_YEAR, MAX77686_RTC_DATE, > + MAX77686_ALARM1_SEC, MAX77686_ALARM1_MIN, MAX77686_ALARM1_HOUR, > + MAX77686_ALARM1_WEEKDAY, MAX77686_ALARM1_MONTH, MAX77686_ALARM1_YEAR, > + MAX77686_ALARM1_DATE, MAX77686_ALARM2_SEC, MAX77686_ALARM2_MIN, > + MAX77686_ALARM2_HOUR, MAX77686_ALARM2_WEEKDAY, MAX77686_ALARM2_MONTH, > + MAX77686_ALARM2_YEAR, MAX77686_ALARM2_DATE, > +}; It is difficult to check for mistakes here. I would prefer direct mapping: [REG_RTC_CONTROLM] = MAX77686_RTC_CONTROLM, .... Rest looks good but I did not check the correctness of mapping above. BR, Krzysztof -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Krzysztof Kozlowski Subject: Re: [PATCH 4/8] rtc: max77686: Add an indirection level to access RTC registers Date: Thu, 21 Jan 2016 10:05:51 +0900 Message-ID: <56A02EEF.2060604@samsung.com> References: <1453310088-29985-1-git-send-email-javier@osg.samsung.com> <1453310088-29985-5-git-send-email-javier@osg.samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Return-path: Received: from mailout3.w1.samsung.com ([210.118.77.13]:34073 "EHLO mailout3.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751028AbcAUBFw (ORCPT ); Wed, 20 Jan 2016 20:05:52 -0500 In-reply-to: <1453310088-29985-5-git-send-email-javier@osg.samsung.com> Sender: linux-samsung-soc-owner@vger.kernel.org List-Id: linux-samsung-soc@vger.kernel.org To: Javier Martinez Canillas , linux-kernel@vger.kernel.org Cc: Kukjin Kim , rtc-linux@googlegroups.com, Chanwoo Choi , Alexandre Belloni , Laxman Dewangan , linux-samsung-soc@vger.kernel.org On 21.01.2016 02:14, Javier Martinez Canillas wrote: > The max77686 driver is generic enough that can be used for other > Maxim RTC IP blocks but these might not have the same registers > layout so instead of accessing the registers directly, add a map > to translate offsets to the real registers addresses for each IP. > > Signed-off-by: Javier Martinez Canillas > --- > > drivers/rtc/rtc-max77686.c | 75 +++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 65 insertions(+), 10 deletions(-) > > diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c > index 441d163dcbeb..7316e41820c7 100644 > --- a/drivers/rtc/rtc-max77686.c > +++ b/drivers/rtc/rtc-max77686.c > @@ -41,6 +41,8 @@ > #define ALARM_ENABLE_SHIFT 7 > #define ALARM_ENABLE_MASK (1 << ALARM_ENABLE_SHIFT) > > +#define REG_RTC_NONE 0xdeadbeef > + > enum { > RTC_SEC = 0, > RTC_MIN, > @@ -55,6 +57,7 @@ enum { > struct rtc_driver_data { > unsigned long delay; > int mask; > + const unsigned int *map; > }; > > struct max77686_rtc_info { > @@ -77,9 +80,53 @@ enum MAX77686_RTC_OP { > MAX77686_RTC_READ, > }; > > +/* These are not registers but just offsets that are mapped to addresses */ > +enum rtc_reg { enum max77686_rtc_reg_offset? > + REG_RTC_CONTROLM = 0, > + REG_RTC_CONTROL, > + REG_RTC_UPDATE0, > + REG_RTC_UPDATE1, > + REG_WTSR_SMPL_CNTL, > + REG_RTC_SEC, > + REG_RTC_MIN, > + REG_RTC_HOUR, > + REG_RTC_WEEKDAY, > + REG_RTC_MONTH, > + REG_RTC_YEAR, > + REG_RTC_DATE, > + REG_ALARM1_SEC, > + REG_ALARM1_MIN, > + REG_ALARM1_HOUR, > + REG_ALARM1_WEEKDAY, > + REG_ALARM1_MONTH, > + REG_ALARM1_YEAR, > + REG_ALARM1_DATE, > + REG_ALARM2_SEC, > + REG_ALARM2_MIN, > + REG_ALARM2_HOUR, > + REG_ALARM2_WEEKDAY, > + REG_ALARM2_MONTH, > + REG_ALARM2_YEAR, > + REG_ALARM2_DATE, > + REG_RTC_END, > +}; > + A short comment what is mapped into what would be appreciated. > +static const unsigned int max77686_map[REG_RTC_END] = { > + MAX77686_RTC_CONTROLM, MAX77686_RTC_CONTROL, MAX77686_RTC_UPDATE0, > + REG_RTC_NONE, MAX77686_WTSR_SMPL_CNTL, MAX77686_RTC_SEC, > + MAX77686_RTC_MIN, MAX77686_RTC_HOUR, MAX77686_RTC_WEEKDAY, > + MAX77686_RTC_MONTH, MAX77686_RTC_YEAR, MAX77686_RTC_DATE, > + MAX77686_ALARM1_SEC, MAX77686_ALARM1_MIN, MAX77686_ALARM1_HOUR, > + MAX77686_ALARM1_WEEKDAY, MAX77686_ALARM1_MONTH, MAX77686_ALARM1_YEAR, > + MAX77686_ALARM1_DATE, MAX77686_ALARM2_SEC, MAX77686_ALARM2_MIN, > + MAX77686_ALARM2_HOUR, MAX77686_ALARM2_WEEKDAY, MAX77686_ALARM2_MONTH, > + MAX77686_ALARM2_YEAR, MAX77686_ALARM2_DATE, > +}; It is difficult to check for mistakes here. I would prefer direct mapping: [REG_RTC_CONTROLM] = MAX77686_RTC_CONTROLM, .... Rest looks good but I did not check the correctness of mapping above. BR, Krzysztof