Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1427368 > unrolled thread

[PATCH v2 00/17] DS1341 support and code cleanup

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 00/17] DS1341 support and code cleanup Andrey Smirnov <andrew.smirnov@gmail.com> - 2016-06-21 09:20 +0200
    Re: [PATCH v2 00/17] DS1341 support and code cleanup Alexandre Belloni <alexandre.belloni@free-electrons.com> - 2016-06-21 23:40 +0200
      Re: [PATCH v2 00/17] DS1341 support and code cleanup Andrey Smirnov <andrew.smirnov@gmail.com> - 2016-06-22 02:00 +0200

#1427368 — [PATCH v2 00/17] DS1341 support and code cleanup

Hi everyone,

This set is a v2 of the DS1307 driver patches. Changes since v1:

 - Devicetree bindings are separated into a separate commit

 - Device tree properties now have vendor specific prefixes and
   documenatation explicitly stating their type

 - Three more patches, with improvements to rtctest.c, are added to
   the patchset

Any feedback is appreciated.

Thank you,
Andrey Smirnov

Andrey Smirnov (17):
  RTC: ds1307: Add DS1341 variant
  RTC: ds1307: Disable square wave and timers as default
  RTC: ds1307: Add devicetree bindings for DS1341
  RTC: ds1307: Add DS1341 specific power-saving options
  RTC: ds1307: Convert ds1307_can_wakeup_device into a predicate
  RTC: ds1307: Convert want_irq into a predicate
  RTC: ds1307: Move chip configuration into a separate routine
  RTC: ds1307: Move chip sanity checking into a separate routine
  RTC: ds1307: Remove register "cache"
  RTC: ds1307: Constify struct ds1307 where possible
  RTC: ds1307: Convert goto to a loop
  RTC: ds1307: Redefine RX8025_REG_* to minimize extra code
  RTC: ds1307: Report oscillator problems more intelligently
  RTC: ds1307: Move last bits of sanity checking out of chip_configure
  RTC: rtctest: Change alarm IRQ support detection
  RTC: rtctest: Change no IRQ detection for RTC_IRQP_READ
  RTC: rtctest: Change no IRQ detection for RTC_IRQP_SET

 .../devicetree/bindings/rtc/dallas,ds1341.txt      |  23 +
 drivers/rtc/rtc-ds1307.c                           | 742 ++++++++++++---------
 tools/testing/selftests/timers/rtctest.c           |  13 +-
 3 files changed, 467 insertions(+), 311 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/rtc/dallas,ds1341.txt

-- 
2.5.5

[toc] | [next] | [standalone]

#1428202

Hi,

On 21/06/2016 at 00:18:22 -0700, Andrey Smirnov wrote :
> Hi everyone,
> 
> This set is a v2 of the DS1307 driver patches. Changes since v1:
> 
>  - Devicetree bindings are separated into a separate commit
> 
>  - Device tree properties now have vendor specific prefixes and
>    documenatation explicitly stating their type
> 
>  - Three more patches, with improvements to rtctest.c, are added to
>    the patchset
> 



> Any feedback is appreciated.
> 
> Thank you,
> Andrey Smirnov
> 
> Andrey Smirnov (17):
>   RTC: ds1307: Add DS1341 variant
>   RTC: ds1307: Disable square wave and timers as default
>   RTC: ds1307: Add devicetree bindings for DS1341
>   RTC: ds1307: Add DS1341 specific power-saving options
>   RTC: ds1307: Convert ds1307_can_wakeup_device into a predicate
>   RTC: ds1307: Convert want_irq into a predicate

I'll have to triple check that one, it breaks in thousand different
ways, every time someone touches that code :)

>   RTC: ds1307: Move chip configuration into a separate routine
>   RTC: ds1307: Move chip sanity checking into a separate routine

I'm not sure about the cleanup in those two patches yet, It moves a lot
of code and the readability improvement is not obvious

>   RTC: ds1307: Remove register "cache"
>   RTC: ds1307: Constify struct ds1307 where possible
>   RTC: ds1307: Convert goto to a loop
>   RTC: ds1307: Redefine RX8025_REG_* to minimize extra code
>   RTC: ds1307: Report oscillator problems more intelligently
>   RTC: ds1307: Move last bits of sanity checking out of chip_configure

>   RTC: rtctest: Change alarm IRQ support detection
>   RTC: rtctest: Change no IRQ detection for RTC_IRQP_READ
>   RTC: rtctest: Change no IRQ detection for RTC_IRQP_SET

I already had patches for that issue in a development tree, I'll see if
they match what I did.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[toc] | [prev] | [next] | [standalone]

#1428268

>>
>> Andrey Smirnov (17):
>>   RTC: ds1307: Add DS1341 variant
>>   RTC: ds1307: Disable square wave and timers as default
>>   RTC: ds1307: Add devicetree bindings for DS1341
>>   RTC: ds1307: Add DS1341 specific power-saving options
>>   RTC: ds1307: Convert ds1307_can_wakeup_device into a predicate
>>   RTC: ds1307: Convert want_irq into a predicate
>
> I'll have to triple check that one, it breaks in thousand different
> ways, every time someone touches that code :)

Wouldn't you agree that this might be an indication that the code is a
bit convoluted and some cleanup is in order? ;-)

>
>>   RTC: ds1307: Move chip configuration into a separate routine
>>   RTC: ds1307: Move chip sanity checking into a separate routine
>
> I'm not sure about the cleanup in those two patches yet, It moves a lot
> of code and the readability improvement is not obvious

OK, I agree that this patch moves a lot of code, and can't really
argue with "not obvious" since that is subjective. Please let me know
what you decide and I'll change v3 appropriately.

Thanks,
Andrey

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web