[PATCH v3 1/5] rtc: mxc_rtc: Driver re work

[Lothar Waßmann]

Hi,

Alexander Shiyan writes:
> > On Sat, Jun 29, 2013 at 12:40:40PM +0400, Alexander Shiyan wrote:
> > > This patch rework mxc_rtc driver.
> > > Major changes have been made:
> > > - Added second clock support (optional) which permit module functionality.
> > > - Implemented support for periodic interrupts.
> > > - Some code have been optimized.
> > > 
> > > Signed-off-by: Alexander Shiyan <shc_work at mail.ru>
> > > ---
> ...
> > > -static inline int is_imx1_rtc(struct rtc_plat_data *data)
> > > -{
> > > -	return data->devtype == IMX1_RTC;
> > > -}
> > 
> > What is wrong with this function?
> 
> All good here. This call is used only once in set_mmss, so no reason
> to have separate function.
> I agree that we can make it in a separate patch, but the optimization of the
> code is specified in the changelog.
> 
That's an inline function anyway. There should be no difference in
code with or without this function. Only difference in source code
readability.

> > > +	pdata->rtc_ops.open		= mxc_rtc_open;
> > > +	pdata->rtc_ops.release		= mxc_rtc_release;
> > > +	pdata->rtc_ops.read_time	= mxc_rtc_read_time;
> > > +	pdata->rtc_ops.set_mmss		= mxc_rtc_set_mmss;
> > > +	pdata->rtc_ops.read_alarm	= mxc_rtc_read_alarm;
> > > +	pdata->rtc_ops.set_alarm	= mxc_rtc_set_alarm;
> > > +	pdata->rtc_ops.alarm_irq_enable	= mxc_rtc_alarm_irq_enable;
> > 
> > So struct rtc_class_ops is embedded into struct rtc_plat_data now. Why
> > is this necessary?
> 
> Just save BSS. Can be moved into cleanup part.
> 
The purpose of platform_data is to convey platform specific
information to drivers, not a general driver local storage.
Thus platform_data should be treated read-only by drivers.


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info at karo-electronics.de
___________________________________________________________