From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752230AbcAUBFy (ORCPT ); Wed, 20 Jan 2016 20:05:54 -0500 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 X-AuditID: cbfec7f5-f79b16d000005389-03-56a02eeedb99 Subject: 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 X-Enigmail-Draft-Status: N1110 Message-id: <56A02EEF.2060604@samsung.com> Date: Thu, 21 Jan 2016 10:05:51 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-version: 1.0 In-reply-to: <1453310088-29985-5-git-send-email-javier@osg.samsung.com> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrJLMWRmVeSWpSXmKPExsVy+t/xq7rv9BaEGVyfw2LRcW0xk8X1L89Z Ld68XcNk8fqFoUX/49fMFkv3rWaxuLxrDpvFjPP7mCz2d3YwOnB6PNl0kdFjz8STbB6bVnWy efQ2v2Pz2NJ/l92jb8sqRo/Pm+QC2KO4bFJSczLLUov07RK4MrYe3sJScFis4umsBewNjHuF uhg5OSQETCS2dV5ggrDFJC7cW8/WxcjFISSwlFGite0DK4TzlFFi+eFrbCBVwgJREp9PNLOA 2CICoRL/Lt5mhChqZpSYemEFK0iCWeAdo8SEPbYgNpuAscTm5UvYIFbISfR2TwJq5uDgFdCS +LvSDCTMIqAq0f3yLNhMUYEIicOdXewgNq+AoMSPyffA4pwC7hInuvYxg7QyC+hJ3L+oBbFJ XmLzmrfMExgFZyHpmIVQNQtJ1QJG5lWMoqmlyQXFSem5RnrFibnFpXnpesn5uZsYIRHydQfj 0mNWhxgFOBiVeHhvXJsfJsSaWFZcmXuIUYKDWUmEV05lQZgQb0piZVVqUX58UWlOavEhRmkO FiVx3pm73ocICaQnlqRmp6YWpBbBZJk4OKUaGFNF9apnG9RMn7jGt+3b5YJCnQfTbW5df8G/ +NJdT8YI25uaFWwTZ6z8ZfAmqsPzxfktvvLnAvU2T1rm+rny1nfGlJYj2nNtgiNUZdXmXXqh FSZ3flNYznvjOWE/WM/43f7cnRey0Gbz9T++ZwL0rQRs1f3V5vm7PF728cc+q7S7gg+2TM2x ZFFiKc5INNRiLipOBAAKArE+jAIAAA== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@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