Re: Re: [PATCH] Remove voodoo code from i2c

[Jon Smirl <jonsmirl@yahoo.com>]

Can this be written as a generic i2c probe function by calling i2c chanel
functions instead of directly calling radeon_gpio_setscl, etc? The i2c chanel
functions will then in turn call radeon_gpio_setscl, etc.

I was under the impression that this code is used to wake up old style DDC chips
in the monitor. Waking them up is a function of the monitor, not the radeon
card. If that is right, this is a monitor problem and this code needs to be in
every DDC driver, not just the radeon one. On the other hand I've tried every
old monitor I have around here and none of them need this code.

--- Kronos <kronos@kronoz.cjb.net> wrote:
> Il Sat, Mar 06, 2004 at 10:55:14AM +1100, Benjamin Herrenschmidt ha scritto: 
> > On Sat, 2004-03-06 at 08:52, Kronos wrote:
> > > Hi,
> > > radeon_probe_i2c_connector contains very strange code that just send
> > > empty start-stop sequencies on the I2C bus. I copied the code from
> > > XFree86, but speaking with I2C gurus the general feeling is that this
> > > code can go away without any harm. I tested it on 3 different setup (all
> > > x86...) and it works well. I don't have any "old" monitor around to
> > > fiddle with though.
> > > 
> > > Oh, if you think that this code should stay I'll send you another patch
> > > to make it more readable.
> > 
> > I'd rather keep the code as close to XFree as possible at this point..
> 
> Ok, what about this (cosmetic) patch:
> 
> --- linux-2.6/drivers/video/aty/radeon_i2c.c.orig	Sat Feb 28 18:11:42 2004
> +++ linux-2.6/drivers/video/aty/radeon_i2c.c	Sat Feb 28 18:49:31 2004
> @@ -174,65 +174,59 @@
>  int radeon_probe_i2c_connector(struct radeonfb_info *rinfo, int conn, u8
> **out_edid)
>  {
>  	u32 reg = rinfo->i2c[conn-1].ddc_reg;
> +	struct radeon_i2c_chan *chan = &rinfo->i2c[conn-1];
>  	u8 *edid = NULL;
>  	int i, j;
>  
>  	OUTREG(reg, INREG(reg) & 
>  			~(VGA_DDC_DATA_OUTPUT | VGA_DDC_CLK_OUTPUT));
>  
> -	OUTREG(reg, INREG(reg) & ~(VGA_DDC_CLK_OUT_EN));
> -	(void)INREG(reg);
> +	radeon_gpio_setscl(chan, 1);
>  
>  	for (i = 0; i < 3; i++) {
>  		/* For some old monitors we need the
>  		 * following process to initialize/stop DDC
>  		 */
> -		OUTREG(reg, INREG(reg) & ~(VGA_DDC_DATA_OUT_EN));
> -		(void)INREG(reg);
> +		radeon_gpio_setsda(chan, 1);
>  		wait_ms(13);
>  
> -		OUTREG(reg, INREG(reg) & ~(VGA_DDC_CLK_OUT_EN));
> -		(void)INREG(reg);
> +		radeon_gpio_setscl(chan, 1);
>  		for (j = 0; j < 5; j++) {
> +			/* Wait for ack */
>  			wait_ms(10);
> -			if (INREG(reg) & VGA_DDC_CLK_INPUT)
> +			if (radeon_gpio_getscl(chan))
>  				break;
>  		}
>  		if (j == 5)
>  			continue;
>  
> -		OUTREG(reg, INREG(reg) | VGA_DDC_DATA_OUT_EN);
> -		(void)INREG(reg);
> +		radeon_gpio_setsda(chan, 0);
>  		wait_ms(15);
> -		OUTREG(reg, INREG(reg) | VGA_DDC_CLK_OUT_EN);
> -		(void)INREG(reg);
> +		radeon_gpio_setscl(chan, 0);
>  		wait_ms(15);
> -		OUTREG(reg, INREG(reg) & ~(VGA_DDC_DATA_OUT_EN));
> -		(void)INREG(reg);
> +		radeon_gpio_setsda(chan, 1);
>  		wait_ms(15);
>  
>  		/* Do the real work */
> -		edid = radeon_do_probe_i2c_edid(&rinfo->i2c[conn-1]);
> +		edid = radeon_do_probe_i2c_edid(chan);
>  
> -		OUTREG(reg, INREG(reg) | 
> -				(VGA_DDC_DATA_OUT_EN | VGA_DDC_CLK_OUT_EN));
> -		(void)INREG(reg);
> +		radeon_gpio_setsda(chan, 0);
> +		radeon_gpio_setscl(chan, 0);
>  		wait_ms(15);
> -		
> -		OUTREG(reg, INREG(reg) & ~(VGA_DDC_CLK_OUT_EN));
> -		(void)INREG(reg);
> +	
> +		radeon_gpio_setscl(chan, 1);
>  		for (j = 0; j < 10; j++) {
> +			/* Wait for ack */
>  			wait_ms(10);
> -			if (INREG(reg) & VGA_DDC_CLK_INPUT)
> +			if (radeon_gpio_getscl(chan))
>  				break;
>  		}
>  
> -		OUTREG(reg, INREG(reg) & ~(VGA_DDC_DATA_OUT_EN));
> -		(void)INREG(reg);
> +		radeon_gpio_setsda(chan, 1);
>  		wait_ms(15);
> -		OUTREG(reg, INREG(reg) |
> -				(VGA_DDC_DATA_OUT_EN | VGA_DDC_CLK_OUT_EN));
> -		(void)INREG(reg);
> +
> +		radeon_gpio_setsda(chan, 0);
> +		radeon_gpio_setscl(chan, 0);
>  		if (edid)
>  			break;
>  	}
> 
> In this way it clearer what's going on.
> 
> Luca
> -- 
> Home: http://kronoz.cjb.net
> "Sei l'unica donna della mia vita".
> (Adamo)
> 
> 
> -------------------------------------------------------
> This SF.Net email is sponsored by: IBM Linux Tutorials
> Free Linux tutorial presented by Daniel Robbins, President and CEO of
> GenToo technologies. Learn everything from fundamentals to system
> administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click
> _______________________________________________
> Linux-fbdev-devel mailing list
> Linux-fbdev-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel


=====
Jon Smirl
jonsmirl@yahoo.com

__________________________________
Do you Yahoo!?
Yahoo! Search - Find what you’re looking for faster
http://search.yahoo.com


-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click