--- In [email protected], Thomas Treyer <[EMAIL PROTECTED]> wrote:
>
> Hi Geert,
> 

Hi Thomas,

> thank you very much for your support. 

No problem, I never thought so many people would be pleased with what
I've done.  It's nice to read this anyhow! :-)

> I have now my first I2C  
> hardware connected, and it runs fine with your driver. I use your  
> driver to control a PCA9532 16 bit I/O expander, which is dedicated  
> to drive LEDs and has pulse-width modulation inside to control the  
> brightness of the LEDs. So you can add the PCA9532 to your list of  
> tested devices.

As of now, it's on my "official list of tested devices"...  ;-)

> I think I understand now where my initial problems came from: When I  
> tested the I2C driver without hardware, I alway got -1 as the return  
> value of ioctl( fd_i2c, _IO( ETRAXI2C_IOCTYPE, I2C_WRITE ),  
> &i2cdata_pca9532 ). Looking in your code this seems to correspond to  
> EI2CBUSNFREE, but of course the bus was free all the time. Both SDA  
> and SCL were terminated with a pull-up resistor. So I expected  
> EI2CWADDRESS instead of EI2CBUSNFREE as the correct error message in  
> case of a missing slave device.

Hmmm... That's strange.  You should indeed receive an EI2CWADDRESS or
EI2CRADDRESS error, not a -1...

> As a last  
> resort I used printk. I know this is not the best debugging style,  
> but it worked. 

Well, once you're in "driverland", this is sometimes the only way out.
 Why do you thing they "invented" the command 'dmesg', to give one
example??? ;-) ;-) ;-)

> Your code produced a correct EI2CWADDRESS, but the  
> return value of ioctl was -1 always.

Phewwwww... Quite a relief...

> My impression is, that ioctl is not able to report your error codes  
> from the kernel space to the user space. In the documentation I found  
> in the Internet the only return values for ioctl are 0 and -1. 

Well, honesty tells me I was not aware of that.  Maybe I should return
the error value as a member of the I2C data structure, then it WILL be
returned to user space, together with all the other data.
You triggered me to solve this issue and I will take care of that.

> Then I  
> remembered, that you plan to write some High-Level drivers for your  
> slave devices, and maybe you are calling the I2C driver from another  
> module inside the kernel. In this case I would expect that the return  
> values are visible from the calling kernel module. Is this assumption  
> correct?

Well, partly.  What is my ultimate goal?  This is to have only the I2C
driver in kernel space.  I don't like to have one kernel module
calling the other one, although I know this has been the original idea
of the initial creators of that file.  Hence the --and that I find a
pity-- ability to call the individual functions that together form an
I2C protocol (like a separate start(), stop(), writebyte() function
etc. ect).  I would never have done this.
After all, it's just the I2C driver who manipulates the hardware, the
pins, so the choice of having this, and only this, in driver land is
obvious...

Therefore, all my other drivers will reside in user space and I will
use the ioctl() function to call the I2C driver.
The drivers I'm planning to make, are PCF8574, PCF8591, TDA8444, LM75,
just to name some of them.
If they all reside in user space, it's much easier/logical to access
them.  The only link to driver land will be the ioctl() call.

This gives me also the opportunity to protect the I2C driver from
multiple calls.  I can very easily use a semaphore to protect
re-entrancy.  This will make life much more easy...

But that's my idea.  Other people might have other ideas/meanings
about this.

> Looking at your code I learnt a lot. Nevertheless there is something  
> I do not understand.
> 
> In i2c.c there is the following if-else clause:
> #ifdef CONFIG_ETRAX_I2C_USES_PB_NOT_PB_I2C
> 
> In the if-clause there is a #define for reading SCL:
> #define i2c_scl_is_high() ( ( ( *R_PORT_PB_READ & ( 1 << SCLBIT ) ) )  
>  >> SCLBIT )
> 
> In the else-clause there is no #define to read SCL. Maybe this can be  
> a problem as soon as someone activates the kernel option  
> CONFIG_ETRAX_I2C_USES_PB.

Well, this proves that you have looked very carefully to the code. 
This is a very good remark.  And the explanation as to why I did it
like that, is very simple:

The code to check the state of the SCL line was not there in the very
beginning.  But then I was not able to return a "bus not free" error,
as long as I could only check the level of the SDA line and not of the
SCL line.

Since I was anyhow not using the "else" part of that #define, I simply
"forgot" to add it over there.
That proves I didn't test the usage of the configuration
"CONFIG_ETRAX_I2C_USES_PB".
And I didn't spend time on it, since I remember I read somewhere that
using this configuration was giving problems with the RTC.  But I'm
not fully sure of this any more.

Nevertheless, you're right and for completeness, the SCL level check
should also be in the "#else" part.  Thumbs up for you!

> In the near future I will add more slave devices to the bus and will  
> report my test results to you. Planned are DS1302 and PCF8591.

Looking forward to see the results.

> Again, thank you very much for your contribution of this driver.

As already said, the pleasure is mine.  I anyhow needed it for myself,
so why not let other people enjoy it too?

Best rgds,

--Geert

Reply via email to