* [PATCH] rtc: ds1307: fix ds1339 wakealarm support
@ 2018-10-22 12:38 Soeren Moch
2018-10-22 13:57 ` Alexandre Belloni
0 siblings, 1 reply; 5+ messages in thread
From: Soeren Moch @ 2018-10-22 12:38 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Soeren Moch, stable, Andrea Greco, Alessandro Zummo, linux-rtc,
linux-kernel
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@vger.kernel.org
Signed-off-by: Soeren Moch <smoch@web.de>
--
Cc: Andrea Greco <a.greco@4sigma.it>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: linux-rtc@vger.kernel.org
Cc: linux-kernel@vger.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,
};
static int ds1307_probe(struct i2c_client *client,
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] rtc: ds1307: fix ds1339 wakealarm support
2018-10-22 12:38 [PATCH] rtc: ds1307: fix ds1339 wakealarm support Soeren Moch
@ 2018-10-22 13:57 ` Alexandre Belloni
2018-10-22 14:18 ` Soeren Moch
0 siblings, 1 reply; 5+ messages in thread
From: Alexandre Belloni @ 2018-10-22 13:57 UTC (permalink / raw)
To: Soeren Moch
Cc: stable, Andrea Greco, Alessandro Zummo, linux-rtc, linux-kernel
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@vger.kernel.org
> Signed-off-by: Soeren Moch <smoch@web.de>
> --
> Cc: Andrea Greco <a.greco@4sigma.it>
> Cc: Alessandro Zummo <a.zummo@towertech.it>
> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Cc: linux-rtc@vger.kernel.org
> Cc: linux-kernel@vger.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.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] rtc: ds1307: fix ds1339 wakealarm support
2018-10-22 13:57 ` Alexandre Belloni
@ 2018-10-22 14:18 ` Soeren Moch
2018-10-22 15:52 ` Alexandre Belloni
0 siblings, 1 reply; 5+ messages in thread
From: Soeren Moch @ 2018-10-22 14:18 UTC (permalink / raw)
To: Alexandre Belloni
Cc: stable, Andrea Greco, Alessandro Zummo, linux-rtc, linux-kernel
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@vger.kernel.org
>> Signed-off-by: Soeren Moch <smoch@web.de>
>> --
>> Cc: Andrea Greco <a.greco@4sigma.it>
>> Cc: Alessandro Zummo <a.zummo@towertech.it>
>> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
>> Cc: linux-rtc@vger.kernel.org
>> Cc: linux-kernel@vger.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?
If we really need this, can we apply this partial revert as regression
fix (also for stable) and implement the address space limitation separately?
Thanks,
Soeren
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] rtc: ds1307: fix ds1339 wakealarm support
2018-10-22 14:18 ` Soeren Moch
@ 2018-10-22 15:52 ` Alexandre Belloni
2018-10-22 17:02 ` Soeren Moch
0 siblings, 1 reply; 5+ messages in thread
From: Alexandre Belloni @ 2018-10-22 15:52 UTC (permalink / raw)
To: Soeren Moch
Cc: stable, Andrea Greco, Alessandro Zummo, linux-rtc, linux-kernel
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@vger.kernel.org
> >> Signed-off-by: Soeren Moch <smoch@web.de>
> >> --
> >> Cc: Andrea Greco <a.greco@4sigma.it>
> >> Cc: Alessandro Zummo <a.zummo@towertech.it>
> >> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
> >> Cc: linux-rtc@vger.kernel.org
> >> Cc: linux-kernel@vger.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.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] rtc: ds1307: fix ds1339 wakealarm support
2018-10-22 15:52 ` Alexandre Belloni
@ 2018-10-22 17:02 ` Soeren Moch
0 siblings, 0 replies; 5+ messages in thread
From: Soeren Moch @ 2018-10-22 17:02 UTC (permalink / raw)
To: Alexandre Belloni
Cc: stable, Andrea Greco, Alessandro Zummo, linux-rtc, linux-kernel
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@vger.kernel.org
>>>> Signed-off-by: Soeren Moch <smoch@web.de>
>>>> --
>>>> Cc: Andrea Greco <a.greco@4sigma.it>
>>>> Cc: Alessandro Zummo <a.zummo@towertech.it>
>>>> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
>>>> Cc: linux-rtc@vger.kernel.org
>>>> Cc: linux-kernel@vger.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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-10-22 17:02 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-10-22 12:38 [PATCH] rtc: ds1307: fix ds1339 wakealarm support Soeren Moch
2018-10-22 13:57 ` Alexandre Belloni
2018-10-22 14:18 ` Soeren Moch
2018-10-22 15:52 ` Alexandre Belloni
2018-10-22 17:02 ` Soeren Moch
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).