Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754418AbbGUKPk (ORCPT ); Tue, 21 Jul 2015 06:15:40 -0400 Received: from mail1.bemta3.messagelabs.com ([195.245.230.166]:41845 "EHLO mail1.bemta3.messagelabs.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752247AbbGUKPi convert rfc822-to-8bit (ORCPT ); Tue, 21 Jul 2015 06:15:38 -0400 X-Env-Sender: stwiss.opensource@diasemi.com X-Msg-Ref: server-13.tower-39.messagelabs.com!1437473731!23473564!1 X-Originating-IP: [94.185.165.51] X-StarScan-Received: X-StarScan-Version: 6.13.16; banners=-,-,- X-VirusChecked: Checked From: "Opensource [Steve Twiss]" To: Alexandre Belloni CC: Alessandro Zummo , DEVICETREE , LINUXINPUT , LINUXKERNEL , RTCLINUX , David Dajun Chen , "Ian Campbell" , Kumar Gala , Lee Jones , Mark Rutland , "Pawel Moll" , Rob Herring , Samuel Ortiz , Support Opensource Subject: RE: [PATCH V2 3/3] rtc: da9063: Add DA9062 RTC capability to DA9063 RTC driver Thread-Topic: [PATCH V2 3/3] rtc: da9063: Add DA9062 RTC capability to DA9063 RTC driver Thread-Index: AQHQw5n4gUFx5a7hhkiPN3X29ybnB53loI+AgAATe2A= Date: Tue, 21 Jul 2015 10:15:29 +0000 Message-ID: <6ED8E3B22081A4459DAC7699F3695FB7014B254064@SW-EX-MBX02.diasemi.com> References: <93f0906ed7db18b893385b60a43b02f248dfe9a6.1437471790.git.stwiss.opensource@diasemi.com> <20150721100151.GC2853@piout.net> In-Reply-To: <20150721100151.GC2853@piout.net> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.20.26.77] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2166 Lines: 68 On 21 July 2015 11:02, Alexandre Belloni wrote: Hi Alexandre, Thanks for your help on this I will send patch v3 with the following changes ... > On 21/07/2015 at 10:43:10 +0100, S Twiss wrote : > > - if (val & (DA9063_E_ALARM)) > > + if (val & (config->rtc_event_alarm_mask)) > unneeded parenthesis here changed to this: - if (val & (config->rtc_event_alarm_mask)) + if (val & config->rtc_event_alarm_mask) > > + if (of_device_is_compatible(pdev->dev.of_node, "dlg,da9063-rtc")) { > > + struct da9063 *chip = dev_get_drvdata(pdev->dev.parent); > > + > > + if (chip->variant_code == PMIC_DA9063_AD) > > + rtc->config = &da9063_ad_regs; > > + } else > > + rtc->config = match->data; > > Both arms of the if should use braces. > Also, what happens if it is a da9063, BB variant? I guess rtc->config > will not be set. > maybe you should write: > > rtc->config = match->data; > if (of_device_is_compatible(pdev->dev.of_node, "dlg,da9063-rtc")) { > struct da9063 *chip = dev_get_drvdata(pdev->dev.parent); > > if (chip->variant_code == PMIC_DA9063_AD) > rtc->config = &da9063_ad_regs; > } Sure, that is more efficient. No need for an else and explicitly defaulting the BB register mappings. + rtc->config = match->data; if (of_device_is_compatible(pdev->dev.of_node, "dlg,da9063-rtc")) { struct da9063 *chip = dev_get_drvdata(pdev->dev.parent); if (chip->variant_code == PMIC_DA9063_AD) rtc->config = &da9063_ad_regs; - } else - rtc->config = match->data; + } > > MODULE_AUTHOR("S Twiss "); > > MODULE_DESCRIPTION("Real time clock device driver for Dialog DA9063"); > > -MODULE_LICENSE("GPL v2"); > > +MODULE_LICENSE("GPL"); > > This should be part of the license change. Sure: -MODULE_LICENSE("GPL"); +MODULE_LICENSE("GPL v2"); Regards, Steve -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/