[U-Boot] [PATCH] video: ct6900: Add an option to skip video initialization

[U-Boot] [PATCH] video: ct6900: Add an option to skip video initialization

[Stefan Roese]

This patch adds an option to skip the video initialization on the
ct6900. This is needed for the CPCI750 which can be built as CPCI
host and adapter/target board. And the adapter board can't access the
video cards.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Anatolij Gustschin <agust@denx.de>
---
 drivers/video/ct69000.c |   16 +++++++++++++++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/drivers/video/ct69000.c b/drivers/video/ct69000.c
index ae219cc..6a5243b 100644
--- a/drivers/video/ct69000.c
+++ b/drivers/video/ct69000.c
@@ -964,6 +964,16 @@ static struct pci_device_id supported[] = {
 	{}
 };
 
+/*
+ * Implement a weak default function for boards that optionally
+ * need to skip the video initialization.
+ */
+int __board_video_skip(void)
+{
+	return 0;
+}
+int board_video_skip(void) __attribute__((weak, alias("__board_video_skip")));
+
 /*******************************************************************************
 *
 * Init video chip
@@ -982,8 +992,12 @@ video_hw_init (void)
 	struct ctfb_res_modes *res_mode;
 	struct ctfb_res_modes var_mode;
 	struct ctfb_chips_properties *chips_param;
-	/* Search for video chip */
 
+	/* Check if video initialization should be skipped */
+	if (board_video_skip())
+		return NULL;
+
+	/* Search for video chip */
 	if ((devbusfn = pci_find_devices (supported, 0)) < 0) {
 #ifdef CONFIG_VIDEO_ONBOARD
 		printf ("Video: Controller not found !\n");
-- 
1.6.2.5

[U-Boot] [PATCH] video: ct6900: Add an option to skip video initialization

[Wolfgang Denk]

Dear Stefan Roese,

In message <1242278732-23803-1-git-send-email-sr@denx.de> you wrote:
> This patch adds an option to skip the video initialization on the
> ct6900. This is needed for the CPCI750 which can be built as CPCI
> host and adapter/target board. And the adapter board can't access the
> video cards.

Hm... why do you change the code of the viseo driver then? This change
should be local to the CPCI750 board only.

> +/*
> + * Implement a weak default function for boards that optionally
> + * need to skip the video initialization.
> + */
> +int __board_video_skip(void)
> +{
> +	return 0;
> +}
> +int board_video_skip(void) __attribute__((weak, alias("__board_video_skip")));
> +
>  /*******************************************************************************
>  *
>  * Init video chip
> @@ -982,8 +992,12 @@ video_hw_init (void)
>  	struct ctfb_res_modes *res_mode;
>  	struct ctfb_res_modes var_mode;
>  	struct ctfb_chips_properties *chips_param;
> -	/* Search for video chip */
>  
> +	/* Check if video initialization should be skipped */
> +	if (board_video_skip())
> +		return NULL;

This makes no sense  to  me.  Instead  of  adding  a  (basicly  board
specific,  even if you hide it with the weak implementation) shortcut
to video_hw_init() you should not initialize the video driver on this
board at all, leaving the video driver code unchanged.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"If the code and  the  comments  disagree,  then  both  are  probably
wrong."                                                - Norm Schryer

[U-Boot] [PATCH] video: ct6900: Add an option to skip video initialization

[Stefan Roese]

Hi Wolfgang,

On Thursday 14 May 2009 15:37:15 Wolfgang Denk wrote:
> > This patch adds an option to skip the video initialization on the
> > ct6900. This is needed for the CPCI750 which can be built as CPCI
> > host and adapter/target board. And the adapter board can't access the
> > video cards.
>
> Hm... why do you change the code of the viseo driver then? This change
> should be local to the CPCI750 board only.

I would love to do it this way. It's not possible though. At least I don't see 
such a solution.

> > +/*
> > + * Implement a weak default function for boards that optionally
> > + * need to skip the video initialization.
> > + */
> > +int __board_video_skip(void)
> > +{
> > +	return 0;
> > +}
> > +int board_video_skip(void) __attribute__((weak,
> > alias("__board_video_skip"))); +
> > 
> > /************************************************************************
> >******* *
> >  * Init video chip
> > @@ -982,8 +992,12 @@ video_hw_init (void)
> >  	struct ctfb_res_modes *res_mode;
> >  	struct ctfb_res_modes var_mode;
> >  	struct ctfb_chips_properties *chips_param;
> > -	/* Search for video chip */
> >
> > +	/* Check if video initialization should be skipped */
> > +	if (board_video_skip())
> > +		return NULL;
>
> This makes no sense  to  me.  Instead  of  adding  a  (basicly  board
> specific,  even if you hide it with the weak implementation) shortcut
> to video_hw_init() you should not initialize the video driver on this
> board at all, leaving the video driver code unchanged.

Then please let me know how this can be accomplished. The CPCI750 uses the 
same U-Boot image both for the video-enabled CPCI-host version and for the 
video-disabled CPCI-adapter version. The video driver is not called from 
within the CPCI750 board files. It's called from drivers/video/cfb_console.c. 

Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH] video: ct6900: Add an option to skip video initialization

[Wolfgang Denk]

Dear Stefan Roese,

In message <200905141553.20682.sr@denx.de> you wrote:
>
> I would love to do it this way. It's not possible though. At least I don't see 
> such a solution.

Of course it's possible. Anything is possible in software :-)

> Then please let me know how this can be accomplished. The CPCI750 uses the 
> same U-Boot image both for the video-enabled CPCI-host version and for the 
> video-disabled CPCI-adapter version. The video driver is not called from 
> within the CPCI750 board files. It's called from drivers/video/cfb_console.c. 

And who calls the code in drivers/video/cfb_console.c ? I think this
is the drv_video_init() call in common/devices.c, right?

If this should be the highest level  in  the  call  chain,  then  any
conditional  calling should be done here. But - don't we already have
a board_video_init()  which  could  return  proper  status  to  abort
initialization?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
God grant me the senility to accept the things I cannot  change,  The
frustration  to  try to change things I cannot affect, and the wisdom
to tell the difference.

[U-Boot] [PATCH] video: ct6900: Add an option to skip video initialization

[Stefan Roese]

On Thursday 14 May 2009 16:02:00 Wolfgang Denk wrote:
> > Then please let me know how this can be accomplished. The CPCI750 uses
> > the same U-Boot image both for the video-enabled CPCI-host version and
> > for the video-disabled CPCI-adapter version. The video driver is not
> > called from within the CPCI750 board files. It's called from
> > drivers/video/cfb_console.c.
>
> And who calls the code in drivers/video/cfb_console.c ? I think this
> is the drv_video_init() call in common/devices.c, right?

Yes.

> If this should be the highest level  in  the  call  chain,  then  any
> conditional  calling should be done here.

OK, I can move the conditional calling to this level.

> But - don't we already have
> a board_video_init()  which  could  return  proper  status  to  abort
> initialization?

This is only implemented for some video drivers (SM501, SED13806, MB826xx) to 
return the base address for the graphics chip. The PCI drivers like the ct6900 
don't use this interface. Here we have other means to get the base address.

I'll move the conditional calling to the highest level and resubmit.

Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH] video: ct6900: Add an option to skip video initialization

[Wolfgang Denk]

Dear Stefan Roese,

In message <200905141613.20127.sr@denx.de> you wrote:
>
> > If this should be the highest level  in  the  call  chain,  then  any
> > conditional  calling should be done here.
> 
> OK, I can move the conditional calling to this level.

I'm not sure.

> > But - don't we already have
> > a board_video_init()  which  could  return  proper  status  to  abort
> > initialization?
> 
> This is only implemented for some video drivers (SM501, SED13806, MB826xx) to 
> return the base address for the graphics chip. The PCI drivers like the ct6900 
> don't use this interface. Here we have other means to get the base address.
> 
> I'll move the conditional calling to the highest level and resubmit.

I'd probably rahter see that you add a board_video_init() to the
ct6900 driver, and have this bail out.

Anatolij, what do you thik is the best solution here?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
[Braddock:] Mr. Churchill, you are drunk.
[Churchill:] And you madam, are ugly.  But I shall be sober tomorrow.

[U-Boot] [PATCH] video: ct6900: Add an option to skip video initialization

[Stefan Roese]

On Thursday 14 May 2009 17:01:46 Wolfgang Denk wrote:
> > > If this should be the highest level  in  the  call  chain,  then  any
> > > conditional  calling should be done here.
> >
> > OK, I can move the conditional calling to this level.
>
> I'm not sure.
>
> > > But - don't we already have
> > > a board_video_init()  which  could  return  proper  status  to  abort
> > > initialization?
> >
> > This is only implemented for some video drivers (SM501, SED13806,
> > MB826xx) to return the base address for the graphics chip. The PCI
> > drivers like the ct6900 don't use this interface. Here we have other
> > means to get the base address.
> >
> > I'll move the conditional calling to the highest level and resubmit.
>
> I'd probably rahter see that you add a board_video_init() to the
> ct6900 driver, and have this bail out.

And what should this function return? The base address is already available in 
the driver. Should we move this to all boards using this driver? I don't think 
so.

> Anatolij, what do you thik is the best solution here?

Yes? What's your preference here?

Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH] video: ct6900: Add an option to skip video initialization

[Anatolij Gustschin]

Hello Wolfgang, Stefan,

Wolfgang Denk wrote:

> In message <200905141613.20127.sr@denx.de> you wrote:
>>> If this should be the highest level  in  the  call  chain,  then  any
>>> conditional  calling should be done here.
>> OK, I can move the conditional calling to this level.
> 
> I'm not sure.
> 
>>> But - don't we already have
>>> a board_video_init()  which  could  return  proper  status  to  abort
>>> initialization?
>> This is only implemented for some video drivers (SM501, SED13806, MB826xx) to 
>> return the base address for the graphics chip. The PCI drivers like the ct6900 
>> don't use this interface. Here we have other means to get the base address.
>>
>> I'll move the conditional calling to the highest level and resubmit.
> 
> I'd probably rahter see that you add a board_video_init() to the
> ct6900 driver, and have this bail out.

this could be done, but the problem here isn't ct6900 driver specific
  
> Anatolij, what do you thik is the best solution here?

i think, there could be other cases were the chip auto-probing
in an init routine can't be accomplished, e.g. some pluggable (not
PCI) graphic extension modules. If we move this bail out code to
the highest level in the call chain ( here i mean to the top of
drv_video_init() in drivers/video/cfb_console.c, we can cover other
similar cases by providing board_video_skip() in the board code.

Best regards,
Anatolij

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de

[U-Boot] [PATCH] video: ct6900: Add an option to skip video initialization

[Anatolij Gustschin]

Stefan Roese wrote:
> On Thursday 14 May 2009 17:01:46 Wolfgang Denk wrote:
>>>> If this should be the highest level  in  the  call  chain,  then  any
>>>> conditional  calling should be done here.
>>> OK, I can move the conditional calling to this level.
>> I'm not sure.
>>
>>>> But - don't we already have
>>>> a board_video_init()  which  could  return  proper  status  to  abort
>>>> initialization?
>>> This is only implemented for some video drivers (SM501, SED13806,
>>> MB826xx) to return the base address for the graphics chip. The PCI
>>> drivers like the ct6900 don't use this interface. Here we have other
>>> means to get the base address.
>>>
>>> I'll move the conditional calling to the highest level and resubmit.
>> I'd probably rahter see that you add a board_video_init() to the
>> ct6900 driver, and have this bail out.
> 
> And what should this function return? The base address is already available in 
> the driver. Should we move this to all boards using this driver? I don't think 
> so.
> 
>> Anatolij, what do you thik is the best solution here?
> 
> Yes? What's your preference here?

i suggest to move the board_video_skip() to the top of
drv_video_init() in drivers/video/cfb_console.c.

What are the arguments against doing it?

Best regards,
Anatolij

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de

[U-Boot] [PATCH] video: ct6900: Add an option to skip video initialization

[Wolfgang Denk]

Dear Anatolij Gustschin,

In message <4A0CBB03.9040009@denx.de> you wrote:
>
> i suggest to move the board_video_skip() to the top of
> drv_video_init() in drivers/video/cfb_console.c.
> 
> What are the arguments against doing it?

Not from my side.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Is truth not truth for all?
	-- Natira, "For the World is Hollow and I have Touched
		   the Sky", stardate 5476.4.

[U-Boot] [PATCH] video: ct6900: Add an option to skip video initialization

[Stefan Roese]

On Friday 15 May 2009 07:36:33 Wolfgang Denk wrote:
> > i suggest to move the board_video_skip() to the top of
> > drv_video_init() in drivers/video/cfb_console.c.
> >
> > What are the arguments against doing it?
>
> Not from my side.

OK, I'll post a new patch in a short while.

Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================