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 <[email protected]>
> ---
>
> 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
