linux-kernel - Re: [PATCH] rtc: ds1307: fix ds1339 wakealarm 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] [day] [month] [year] [list]

Date:   Mon, 22 Oct 2018 19:02:10 +0200
From:   Soeren Moch <smoch@....de>
To:     Alexandre Belloni <alexandre.belloni@...tlin.com>
Cc:     stable@...r.kernel.org, Andrea Greco <a.greco@...gma.it>,
        Alessandro Zummo <a.zummo@...ertech.it>,
        linux-rtc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] rtc: ds1307: fix ds1339 wakealarm support



Am 22.10.2018 um 17:52 schrieb Alexandre Belloni:
> On 22/10/2018 16:18:13+0200, Soeren Moch wrote:
>> Hi,
>>
>> Am 22.10.2018 um 15:57 schrieb Alexandre Belloni:
>>> Hello,
>>>
>>> On 22/10/2018 14:38:12+0200, Soeren Moch wrote:
>>>> Commit 51ed73eb998a1c79a2b0e9bed68f75a8a2c93b9b ("rtc: ds1340: Add support
>>>> for trickle charger.") breaks ds1339 wakealarm support by limiting
>>>> accessible registers. Fix this.
>>>>
>>>> Fixes: 51ed73eb998a ("rtc: ds1340: Add support for trickle charger.")
>>>> Cc: stable@...r.kernel.org
>>>> Signed-off-by: Soeren Moch <smoch@....de>
>>>> --
>>>> Cc: Andrea Greco <a.greco@...gma.it>
>>>> Cc: Alessandro Zummo <a.zummo@...ertech.it>
>>>> Cc: Alexandre Belloni <alexandre.belloni@...tlin.com>
>>>> Cc: linux-rtc@...r.kernel.org
>>>> Cc: linux-kernel@...r.kernel.org
>>>> ---
>>>>  drivers/rtc/rtc-ds1307.c | 1 -
>>>>  1 file changed, 1 deletion(-)
>>>>
>>>> diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
>>>> index 4b2b4627daeb..71396b62dc52 100644
>>>> --- a/drivers/rtc/rtc-ds1307.c
>>>> +++ b/drivers/rtc/rtc-ds1307.c
>>>> @@ -1384,7 +1384,6 @@ static void ds1307_clks_register(struct ds1307 *ds1307)
>>>>  static const struct regmap_config regmap_config = {
>>>>  	.reg_bits = 8,
>>>>  	.val_bits = 8,
>>>> -	.max_register = 0x9,
>>> I would prefer having a different regmap_config for chips that have more
>>> registers. I guess you could put a pointer to a regmap_config in chip_desc.
>>>
>> What looks strange to me, the original patch (according to its
>> description) enabled trickle charger functionality for a single chip.
>> This is something totally different than limiting the register space for
>> all chips that are supported by this driver. So this seems to be an
>> accident to me. And this causes an regression for ds1339, which was
>> supported for years.
>>
>> Are there any advantages for limiting the address space of i2c devices?
> I would think it was done as a facility to debug the feature because
> when max_register is set, regmap will allow you to dump the register
> space.
>
>> If we really need this, can we apply this partial revert as regression
>> fix (also for stable) and implement the address space limitation separately?
>>
> Fine, I'm applying this patch right now.
>
OK, thank you,
Soeren