RE: [PATCH v1 11/16] OMAP3: hwmod DSS: RFBI Move init,exit to driver

2010-10-08 Thread Guruswamy, Senthilvadivu 


 -Original Message-
 From: Thomas Petazzoni [mailto:thomas.petazz...@free-electrons.com]
 Sent: Friday, October 08, 2010 1:17 AM
 To: Guruswamy, Senthilvadivu
 Cc: khil...@deeprootsystems.com; tomi.valkei...@nokia.com; p...@pwsan.com;
 Hiremath, Vaibhav; linux-omap@vger.kernel.org
 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 svad...@ti.com 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 11/16] OMAP3: hwmod DSS: RFBI Move init,exit to driver

2010-10-07 Thread Thomas Petazzoni 
Hello Senthil,

On Wed,  6 Oct 2010 16:44:54 +0530
Guruswamy Senthilvadivu svad...@ti.com 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() ?

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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 11/16] OMAP3: hwmod DSS: RFBI Move init,exit to driver

2010-10-06 Thread Guruswamy Senthilvadivu 
From: Senthilvadivu Guruswamy svad...@ti.com

Move init exit methods to its driver probe,remove.
pdev member has to be maintained by its own drivers.

Signed-off-by: Senthilvadivu Guruswamy svad...@ti.com
---
 drivers/video/omap2/dss/core.c |9 -
 drivers/video/omap2/dss/rfbi.c |   14 +-
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
index 54fe333..cc7a5f1 100644
--- a/drivers/video/omap2/dss/core.c
+++ b/drivers/video/omap2/dss/core.c
@@ -193,12 +193,6 @@ static int omap_dss_probe(struct platform_device *pdev)
 
dss_clk_enable(DSS_CLK_ICK | DSS_CLK_FCK1 | DSS_CLK_54M);
 
-   r = rfbi_init();
-   if (r) {
-   DSSERR(Failed to initialize rfbi\n);
-   goto err_rfbi;
-   }
-
r = dpi_init(pdev);
if (r) {
DSSERR(Failed to initialize dpi\n);
@@ -272,8 +266,6 @@ err_venc:
 err_dispc:
dpi_exit();
 err_dpi:
-   rfbi_exit();
-err_rfbi:
 
return r;
 }
@@ -288,7 +280,6 @@ static int omap_dss_remove(struct platform_device *pdev)
venc_exit();
dispc_exit();
dpi_exit();
-   rfbi_exit();
if (cpu_is_omap34xx()) {
dsi_exit();
sdi_exit();
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;
 }
 
-- 
1.6.3.3

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html