Re: cvs commit: src/sys/pci amdpm.c

On Fri, Dec 16, 2005 at 01:42:25PM +0300, Artemiev Igor wrote:
> > OK, I looked some more, and I doubt the usefullness of the
> > nForce-2/3/4 support in its current shape.  Perhaps I'm mistaken and
> > you can shed a light on this.  :-)
> > The amdpm(4) driver originally supported AMD-756's PMC SMBus 1.0.
> > Later, AMD-8111 support was added.  All of these AMDs seem to support
> > both SMBus 2.0 and SMBus 1.0 interfaces, and the driver uses the SMBus
> > 1.0 interface (offset 0xe0).   The nForce seems to also support SMBus
> > 1.0 interface (offset 0).  At least, the following lm_sensors pages
> > amd AMD-8111 datasheet confirm this:
> > 
> > 	http://www.lm-sensors.org/supported.html
> > 	http://www2.lm-sensors.nu/~lm78/cvs/lm_sensors2/doc/busses/i2c-amd756
> > 	http://www2.lm-sensors.nu/~lm78/cvs/lm_sensors2/doc/busses/i2c-amd8111
> > 
> > Now, the same supported.html page and googling says that nForce2/3/4
> > MCPs all use SMBus 2.0 interface similar to AMD-8111 SMBus 2amd86.0
> > interface.  But we don't support SMBus 2.0 interface in amdpm(4)!
> > (lm_sensors, OTOH, does implement SMBus 2.0.).
> i2c-amd756.c, i2c-amd8111.c and i2c-nforce2.c looks very similar for me.
> Differences between i2c-amd756 and i2c-nforce2.c/i2c-amd8111.c in
> pair ioctl.
> 
i2c-amd8111.c and i2c-nforce2.c are indeed very similar, because they
both use SMBus 2.0 API.  :-)

But i2c-amd756.c, which also handles AMD-8111 SMBus 1.0, and is similar
to FreeBSD's amdpm.c, uses a different API (SMBus 1.0).  Are you sure
the following fragments look similar to you?

http://www2.lm-sensors.nu/~lm78/cvs/browse.cgi/lm_sensors2/kernel/busses/i2c-amd756.c
http://www2.lm-sensors.nu/~lm78/cvs/browse.cgi/lm_sensors2/kernel/busses/i2c-nforce2.c

Let's consider I2C_SMBUS_QUICK+I2C_SMBUS_WRITE case, which is in FreeBSD's
smb(4) spelling is:

     SMB_QUICK_WRITE    The QuickWrite command just issues the device address
                        with write intent to the bus, without transferring any
                        data.

I've added offsets in `//' style comments...

   203	static s32 amd756_access(struct i2c_adapter * adap, u16 addr,
   204			  unsigned short flags, char read_write,
   205			  u8 command, int size, union i2c_smbus_data * data)
   206	{
   207		int i, len;
   208	
   209		/** TODO: Should I supporte the 10-bit transfers? */
   210		switch (size) {
   211		/* TODO: proc call is supported, I'm just not sure what to do here... */
   212		case I2C_SMBUS_QUICK:
   213			outw_p(((addr & 0x7f) << 1) | (read_write & 0x01),
   214			       SMB_HOST_ADDRESS);		// (0x4 + amd756_ioport)
   215			size = AMD756_QUICK;
   216			break;
[...]
   264		/* How about enabling interrupts... */
   265		outw_p(size & GE_CYC_TYPE_MASK, SMB_GLOBAL_ENABLE);	// (0x2 + amd756_ioport) word
   266	
   267		if (amd756_transaction())	/* Error in transaction */
   268			return -1;
   269	
   270		if ((read_write == I2C_SMBUS_WRITE) || (size == AMD756_QUICK))
   271			return 0;



   141	s32 nforce2_access(struct i2c_adapter * adap, u16 addr, unsigned short flags,
   142			char read_write, u8 command, int size,
   143			union i2c_smbus_data * data)
   144	{
   145		struct nforce2_smbus *smbus = adap->algo_data;
   146		unsigned char protocol, pec, temp;
   147		unsigned char len = 0; /* to keep the compiler quiet */
   148		int i;
   149	
   150		protocol = (read_write == I2C_SMBUS_READ) ? NVIDIA_SMB_PRTCL_READ : NVIDIA_SMB_PRTCL_WRITE;
   151		pec = (flags & I2C_CLIENT_PEC) ? NVIDIA_SMB_PRTCL_PEC : 0;
   152	
   153		switch (size) {
   154	
   155			case I2C_SMBUS_QUICK:
   156				protocol |= NVIDIA_SMB_PRTCL_QUICK;
   157				read_write = I2C_SMBUS_WRITE;
   158				break;
[...]
   214		}
   215	
   216		outb_p((addr & 0x7f) << 1, NVIDIA_SMB_ADDR);		// smbus->base + 0x02 (byte)
   217		outb_p(protocol, NVIDIA_SMB_PRTCL);			// smbus->base + 0x00 (byte)
   218	
   219		temp = inb_p(NVIDIA_SMB_STS);				// smbus->base + 0x01 (byte)
   220	
   221		if (~temp & NVIDIA_SMB_STS_DONE) {
   222			udelay(500);
   223			temp = inb_p(NVIDIA_SMB_STS);
   224		}
   225		if (~temp & NVIDIA_SMB_STS_DONE) {
   226			i2c_delay(HZ/100);
   227			temp = inb_p(NVIDIA_SMB_STS);
   228		}
   229	
   230		if ((~temp & NVIDIA_SMB_STS_DONE) || (temp & NVIDIA_SMB_STS_STATUS)) {
   231			printk(KERN_DEBUG "i2c-nforce2.o: SMBus Timeout! (0x%02x)\n",
   232			       temp);
   233			return -1;
   234		}
   235	
   236		if (read_write == I2C_SMBUS_WRITE)
   237			return 0;

See how the offset 0x2 register means completely different things.


Cheers,
-- 
Ruslan Ermilov
ru_at_FreeBSD.org
FreeBSD committer