Re: [PATCH v4 06/10] rtc: max77686: Add max77802 support

From: Javier Martinez Canillas
Date: Tue Jan 26 2016 - 23:37:09 EST


Hello Krzysztof,

On Wed, Jan 27, 2016 at 1:23 AM, Krzysztof Kozlowski
<k.kozlowski@xxxxxxxxxxx> wrote:
> On 27.01.2016 12:36, Javier Martinez Canillas wrote:

[snip]

>>
>> + if (!info->drv_data->separate_i2c_addr)
>> + info->max77686->rtc_regmap = info->max77686->regmap;
>> +
>
> At this stage I don't like the idea of messing with parent's state
> structure. The driver should not modify any of parents data (the best
> way would be to take a pointer to const). In this patch this looks like
> breaking the encapsulation. If the parent is responsible for regmaps,
> then the parent should set rtc_regmap for children (parent also knows
> what type device it is working on).
>
> ...but I am assuming that a new patch will be following this one - the
> patch moving ownership of i2c dummy and regmap to the RTC driver. In
> that case this code makes a lot more sense. Am I thinking correctly?
>

You assumption is correct.

Modifying the parent's state is indeed a layering violation but I did
this to keep the changes self contained in the RTC subsystem and avoid
touching the MFD driver.

But this will be cleaned by Laxman once he moves the regmap
initialization from the MFD driver to the RTC.

> Best regards,
> Krzysztof

Best regards,
Javier