> -----Original Message----- > From: Thomas Petazzoni [mailto:thomas.petazzoni@xxxxxxxxxxxxxxxxxx] > Sent: Friday, October 08, 2010 1:17 AM > To: Guruswamy, Senthilvadivu > Cc: khilman@xxxxxxxxxxxxxxxxxxx; tomi.valkeinen@xxxxxxxxx; paul@xxxxxxxxx; > Hiremath, Vaibhav; linux-omap@xxxxxxxxxxxxxxx > Subject: Re: [PATCH v1 11/16] OMAP3: hwmod DSS: RFBI Move init,exit to > driver > > Hello Senthil, > > On Wed, 6 Oct 2010 16:44:54 +0530 > Guruswamy Senthilvadivu <svadivu@xxxxxx> wrote: > > > diff --git a/drivers/video/omap2/dss/rfbi.c > b/drivers/video/omap2/dss/rfbi.c > > index 23598ea..9bee39d 100644 > > --- a/drivers/video/omap2/dss/rfbi.c > > +++ b/drivers/video/omap2/dss/rfbi.c > > @@ -100,6 +100,7 @@ static int rfbi_convert_timings(struct rfbi_timings > *t); > > static void rfbi_get_clk_info(u32 *clk_period, u32 *max_clk_div); > > > > static struct { > > + struct platform_device *pdev; > > void __iomem *base; > > > > unsigned long l4_khz; > > @@ -142,11 +143,22 @@ static inline u32 rfbi_read_reg(const struct > rfbi_reg idx) > > /* RFBI HW IP initialisation */ > > static int omap_rfbihw_probe(struct platform_device *pdev) > > { > > - return 0; > > + int r; > > + rfbi.pdev = pdev; > > + > > + r = rfbi_init(); > > + if (r) { > > + DSSERR("Failed to initialize rfbi\n"); > > + goto err_rfbi; > > + } > > + > > +err_rfbi: > > + return r; > > } > > > > static int omap_rfbihw_remove(struct platform_device *pdev) > > { > > + rfbi_exit(); > > return 0; > > } > > > > Instead of having probe() and remove() functions that call the existing > init() and exit(), why not making your driver look like most Linux > drivers here, and directly do the initialization in probe() and the > cleanup in remove() ? > [Senthil] When I do existing code movement I don't change it one single patch just to make the review concentrate on the place of movement. I could submit additional patches to remove the extra function calls wherever required. > Concerning the added pdev field to the rfbi structure, do you really it > at this point ? > > Thomas > -- > Thomas Petazzoni, Free Electrons > Kernel, drivers, real-time and embedded Linux > development, consulting, training and support. > http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
