On Thu, 30 May 2013 14:14:42 +0400, Sergey Yanovich <ynv...@gmail.com> wrote: > On Wed, 2013-05-29 at 15:53 -0700, Andrew Morton wrote: >> On Tue, 21 May 2013 03:21:30 +0400 Sergey Yanovich <ynv...@gmail.com> >> wrote: >> @@ -321,6 +326,7 @@ static int ds1302_rtc_remove(struct platform_device >> *pdev) >> > { >> > struct rtc_device *rtc = platform_get_drvdata(pdev); >> > >> > + ds1302_writebyte(RTC_ADDR_CTRL, RTC_CMD_WRITE_DISABLE); >> > rtc_device_unregister(rtc); >> > platform_set_drvdata(pdev, NULL); >> >> ds1302_rtc_remove() no longer exists in my tree - it got whittled away >> to nothing by >> http://ozlabs.org/~akpm/mmots/broken-out/rtc-rtc-ds1302-remove-unnecessary-platform_set_drvdata.patch >> and >> http://ozlabs.org/~akpm/mmots/broken-out/drivers-rtc-rtc-ds1302c-remove-empty-function.patch >> >> Perhaps it should be re-added for this? > > There are 2 options. I would be happy with either. > > 1. I've chosen 'probe/remove' to enable/disable write access. > > 2. Another option is to wrap enable/disable around > ds1302_rtc_set_time(). > > IIUC, the former saves a few bytes of memory. However, now, when > ds1302_rtc_remove() is gone, the latter looks better. So I could rewrite > the patch either way.
Option two looks actually safer to me, as it ensures that an unexpected reboot outside of the set_time section doesn't leave write access enabled. You never know what firmware could do while you're not looking... M. -- Who you jivin' with that Cosmik Debris? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/