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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 27 Jan 2016 01:37:02 -0300
From:	Javier Martinez Canillas <javier@...hile0.org>
To:	Krzysztof Kozlowski <k.kozlowski@...sung.com>
Cc:	Javier Martinez Canillas <javier@....samsung.com>,
	Linux Kernel <linux-kernel@...r.kernel.org>,
	Kukjin Kim <kgene@...nel.org>, rtc-linux@...glegroups.com,
	Andi Shyti <andi.shyti@...sung.com>,
	Chanwoo Choi <cw00.choi@...sung.com>,
	Alexandre Belloni <alexandre.belloni@...e-electrons.com>,
	Laxman Dewangan <ldewangan@...dia.com>,
	"linux-samsung-soc@...r.kernel.org" 
	<linux-samsung-soc@...r.kernel.org>
Subject: Re: [PATCH v4 06/10] rtc: max77686: Add max77802 support

Hello Krzysztof,

On Wed, Jan 27, 2016 at 1:23 AM, Krzysztof Kozlowski
<k.kozlowski@...sung.com> 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