Re: [PATCH] DDC2/I2C Support

On Tuesday 06 January 2004 01:16, Kronos wrote:

Il Mon, Jan 05, 2004 at 09:31:02AM -0500, adaplas@pol.net ha scritto:

quoted

1. [fbi2c.c]:  Added preliminary generic DDC2/i2c support (derived from
xf86DDC.c and xf86i2.c)

	This is a configurable option (CONFIG_FB_I2C). 	For now, I only used
"default" timeout values which may not work for all devices.

	Most drivers need to implement only two functions --

		void xxxfb_i2c_getbits(struct fb_info *info, signed long *clock, signed
long *data)
		void xxxfb_i2c_putbits(struct fb_info *info, signed long clock, signed
long data)

In fbi2c.c you are duplicating the i2c layer that's already there in the
kernel. Each driver should  have its set of function to  control SDA and
SCL and then it's only a matter of calling i2c_transfer.

I wrote the i2c stuff for the new radeon driver, look here:
bk://ppc.bkbits.net/linuxppc-2.5-benh
(drivers/video/aty/radeon_i2c.c)

Ok.  See my reply to Ben H.

quoted

2. [fbmon.c]: Consolidated different ways of getting EDID with the
following function:

	unsigned char *fb_get_edid(struct fb_info *info, struct pci_dev *pdev,
int what)

		the parameter "what" can be:
			EDID_DDC2 - get EDID via DDC2/I2C fb_get_edid_from_ddc2()
			EDID_ARCH - get	EDID via BIOS or OF get_EDID_from_{OF|BIOS}()

Hum, I don't like this kind of "multiplexing". I think that EDID
reading (if possibile) should be done inside the driver which knows how
to do it.

Also, why did you remove all the printk's? Ok, show_edid is a bit too
verbose but it's usefull for debugging.

The first time I wrote the edid parser, show_edid() was never meant to have 
all that printk's :-).  It's only preliminary code in preparation for 
extending struct fb_monspecs.  But I'll bring it back, possibly when DEBUG is 
defined.

Tony


-------------------------------------------------------
This SF.net email is sponsored by: IBM Linux Tutorials.
Become an expert in LINUX or just sharpen your skills.  Sign up for IBM's
Free Linux Tutorials.  Learn everything from the bash shell to sys admin.
Click now! http://ads.osdn.com/?ad_id78&alloc_id371&op=click