On 13.01.2016 23:59, Laxman Dewangan wrote:

On Wednesday 13 January 2016 09:55 AM, Laxman Dewangan wrote:

quoted

On Wednesday 13 January 2016 09:58 AM, Krzysztof Kozlowski wrote:

quoted

On 13.01.2016 13:07, Laxman Dewangan wrote:

quoted

On Wednesday 13 January 2016 05:36 AM, Krzysztof Kozlowski wrote:

quoted

That is also fine to me but still I am not comfortable with the config
name and driver file name as this does not suggest the common.

The name does not matter. Really. We have a lot of drivers with a
specific device-like name and supporting different devices. To point
that your argument is invalid - your initial name of driver
"rtc-max77620.c" supported totally different "names": the max77620 and
max20024. It also wasn't suggesting something "common"...

In all config string, I have mentioned the MAX20024.

quoted

With my approach we are not developing common think neither. We just
want to extend/re-use existing max77686 (or max77802) driver for new
devices. Just like everywhere else.

OK, fine to me if this is acceptable.
I will drop this rtc patch form this series and work in max77686
driver to modify first and once merged, use this config on my defconfig.

Here, MAX686 RTC driver needs two regmap handle, one for the rtc_regmap
and other for STATUS2 register access.

        ret = regmap_read(info->max77686->regmap, MAX77686_REG_STATUS2,
&val);
        if (ret < 0) {
                dev_err(info->dev, "%s:%d fail to read status2 reg(%d)\n",
                                __func__, __LINE__, ret);
                goto out;
        }


We can not have two regmap handle on rtc driver as both regmap (pmic and
rtc) registered with different i2c device.

Also this register should not be accessed by RTC driver if we want to
decouple as this is very much MAX77686 register set.
Do we need this code?
static int max77686_rtc_read_alarm(struct device *dev, struct rtc_wkalrm
*alrm)
{
        struct max77686_rtc_info *info = dev_get_drvdata(dev);
        u8 data[RTC_NR_TIME];
        unsigned int val;
        int i, ret;

::::::::
       alrm->pending = 0;
        ret = regmap_read(info->max77686->regmap, MAX77686_REG_STATUS2,
&val);
        if (ret < 0) {
                dev_err(info->dev, "%s:%d fail to read status2 reg(%d)\n",
                                __func__, __LINE__, ret);
                goto out;
        }

        if (val & (1 << 4)) /* RTCA1 */
                alrm->pending = 1;

}

Most of the drivers set the 'pending' field when reading alarm. Your
original driver did not.

The max77802 does exactly the same (BTW, these should be merged as
well... I'll add this to the TODO list) so I think this is necessary.

BR,
Krzysztof