2010/10/14 Jonathan Cameron <jic23@xxxxxxxxx>:
> On 10/14/10 11:59, matthias wrote:
>> Hi,
>>
>> 2010/10/14 Jonathan Cameron <jic23@xxxxxxxxx>:
>>> On 09/29/10 17:50, matthias wrote:
>>>> Hi Jonathan,
>>>>
>>>> just FYI, I've not too much time right now, but did a quick test with
>>>> commenting the bit_per_words statements in read/write.
>>>> The driver seems to work.
>>>> So at least some good news.
>>>>
>>> Given your spi fix is now in, are you happy to ack this patch?
>>
>> I still have some issues, but I found yesterday. But I'm not sure,
>> maybe the source is my board.
>> I have three adis16255 gyros on my board.
>> I'm only able to load the iio driver once, the second time it fails in
>> the second and sometimes on the third gyro inside the
>> adis16260_check_status routine (the status register indicates a SPI
>> error).
>> In my driver sometimes I'm not able to read the scaling register in
>> the right manner, which means the chip does not return the default
>> value.
>>
>> Just let me see, if I can fix this. I'll try to do finish it this
>> week, but it will be critical to do so.
> That would be great. ÂSounds like something very strange is going on.
> Good luck with the debugging.
>>
>>> Also what do you want to do about your driver in staging?
>>
>> I'd prefer to let it in the staging directory until I'm sure the iio
>> implementation works fine. Then we can delete it.
> Agreed. We definitely want to hold off whilst there are things that
> work in your driver but not the IIO one.
>>
>>> Would be nice to clean this up before the merge window.
>>
>> Do you known, when the next merge window will be?
> 3 months typically.
>> Because I also have a barometer driver (bmp085 from bosch sensortec)
>> which I'd like to bring into the iio subsystem.
> Excellent. ÂI had a look at those a while back for a project, but we
> never added a pressure sensor. ÂNice looking bit of kit.
>> But up to now I'm not
>> so familiar with all the defines, trigger, ring implementation. So
>> maybe we leave that for the next merge window.
> Sounds sensible. ÂFrom our point of view, I think most developers now
> work off the staging-next tree anyway (as there is still a fair bit of
> core work going on) so the driver will be 'available' from whenever Greg
> pulls the patch set (typically a couple of days)
>
> Looking forward to that pressure driver and any feedback on the gyro
> patches.
Acked by Matthias Brugger <mensch0815@xxxxxxxxx>
The problems I had was due to the strange pcb layout we have...
Best regards,
Matthias
>
> Don't worry too much about the merge window time scales. ÂIt's just me
> in my role as 'maintainer' hustling things along :)
>
> Jonathan
>>
>> Regards,
>> Matthias
>>
>>>
>>> Jonathan
>>>> Regards,
>>>> Matthias
>>>>
>>>> 2010/9/29 Jonathan Cameron <jic23@xxxxxxxxx>:
>>>>> Hi Matthias
>>>>>
>>>>> Lots of additional cc's as I think I know what the problem is and I
>>>>> think it's an spi issue rather than an IIO one.
>>>>>
>>>>>>
>>>>>> after a long stuggle, I got a working kernel version for my board
>>>>>> running as I need it.
>>>>>> I tried both the staging/adis16255 and the staging/iio/gyro driver.
>>>>>>
>>>>>> The latter doesn't work.
>>>>>> "adis16260 spi1.1: problem when reading 16 bit register 0x34
>>>>>> iio device0: disable irq failed"
>>>>> That is the first actual comms with the chip, so it is likely to be a
>>>>> general issue rather than due to that particular call.
>>>>>>
>>>>>> A quick look in the code of staging/adis16255 and the data sheet tells
>>>>>> me, that you have to read from the higher register address but we read
>>>>>> from the lower one.
>>>>>> So I changed the value to 0x35, but it doesn't work either.
>>>>> Quoting from the data sheet (it is present on the sheets for both chips).
>>>>>
>>>>> "Each register has two addresses (upper, lower), but either one can be used
>>>>> to access its entire 16 bits of data."
>>>>>
>>>>> It could be there is something special about that register I'm not seeing
>>>>> though...
>>>>>>
>>>>>> Well in the end I copied "my" read version from staging/adis16255 and
>>>>>> put a read call in the probe function (because it is the easiest way
>>>>>> to get spi_device structure).
>>>>>> Then it seems to work, with higher and lower byte.
>>>>> Ok, that gives us something to work against which is very helpful!
>>>>>> So my conclusion is, that something went wrong when you casacade and
>>>>>> discascade (sorry for the poor english), from spi_device through
>>>>>> adis16260_state to device.
>>>>>> Sounds stange to me, as it works with the adis16260 chip, right?
>>>>> As far as I know. It's possible something strange happened in a merge
>>>>> at some point though.
>>>>>> So maybe it's because I use avr32 architecture?
>>>>>
>>>>> Having done a bit of digging and made sure that (up to the fact I don't
>>>>> have the part) everything runs normally on my board, I'm beginning to
>>>>> suspect you are correct. There are some subtle differences in the setup
>>>>> between your code and Barry's.
>>>>>
>>>>> Looking about, the avr32's seem to use the atmel_spi driver?
>>>>>
>>>>> Ah got it (I think)...
>>>>>
>>>>> In drivers/spi/atmel_spi.c atmel_spi_transfer we have:
>>>>>
>>>>> Â Â Â Â Â Â Â Â/* FIXME implement these protocol options!! */
>>>>> Â Â Â Â Â Â Â Âif (xfer->bits_per_word || xfer->speed_hz) {
>>>>> Â Â Â Â Â Â Â Â Â Â Â Âdev_dbg(&spi->dev, "no protocol options yet\n");
>>>>> Â Â Â Â Â Â Â Â Â Â Â Âreturn -ENOPROTOOPT;
>>>>> Â Â Â Â Â Â Â Â}
>>>>>
>>>>> Personally I'd have at least sanity checked if the parameters were trying
>>>>> to change anything before dumping out. ÂAlso, surely that's a case for
>>>>> something screaming a little louder given it is an out and out device
>>>>> killing problem? I think the default is 8 bit? ÂI think the reason it
>>>>> is in Barry's driver is a legacy issue to do with the fact that these
>>>>> are actually 16bit transfers pretending to be 8 bits ones... ÂActually
>>>>> now I look at it, I'm not sure why he didn't use 16 bit ones in that
>>>>> function in the first place!
>>>>>
>>>>> Not sure we need it in our drivers, but I'm guessing there may be some
>>>>> spi master driver out there somewhere that defaults to something other than
>>>>> 8 bit? (have vague recollection of seeing an email about this... perhaps
>>>>> someone who plays more with spi bus drivers?)
>>>>>
>>>>>>
>>>>>> So I don't know if we should dig deeper. The problem is, that I don't
>>>>>> have too much time to do it...
>>>>> Sorry it is proving such a pain for you to test!
>>>>>> What do you think?
>>>>> I have cc'd spi people and the atmel_spi maintainer to see what we think
>>>>> is the correct fix for this. For the purposes of this discussion
>>>>> (though it's isn't quite what Matthias is working with) the driver in
>>>>> question is drivers/staging/iio/gyro/adis16260_core.c
>>>>>
>>>>>
>>>>> Thanks again for testing.
>>>>>
>>>>> Jonathan
>>>>>
>>>>>> Regards,
>>>>>> Matthias
>>>>>>
>>>>>> 2010/9/27 Jonathan Cameron <jic23@xxxxxxxxx>:
>>>>>>> On 09/18/10 16:48, matthias wrote:
>>>>>>>> Hi Jonathan,
>>>>>>>>
>>>>>>>> sorry for not answering yet. I was on vacation and next week I won't
>>>>>>>> be able to test the driver. Will try to do it asap....
>>>>>>>>
>>>>>>>> Matthias
>>>>>>>
>>>>>>> Hi Matthias,
>>>>>>>
>>>>>>> I'm afraid quite a bit has changed over the last few weeks. Feel free to test
>>>>>>> this patch set. ÂThe changes since then are merely renames of a couple of
>>>>>>> attributes and a lot of stuff for event handling that doesn't effect this
>>>>>>> driver.
>>>>>>>
>>>>>>> If not, I'm hosting a *temporary* git tree with all my various queued up changes
>>>>>>> at:
>>>>>>>
>>>>>>> http://git.kernel.org/?p=linux/kernel/git/jic23/iio_temp.git
>>>>>>>
>>>>>>> Seems excessive to post this set again until I hear back from you!
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Jonathan
>>>>>>>
>>>>>>> p.s. Switched to drivers@xxxxxxxxxx address as that now seems to work.
>>>>>>>
>>>>>>>>
>>>>>>>> 2010/9/18 Jonathan Cameron <jic23@xxxxxxxxx>:
>>>>>>>>> On 09/06/10 16:16, Jonathan Cameron wrote:
>>>>>>>>>> Mainly a rebase, but a couple of attribute naming fixes as well.
>>>>>>>>>>
>>>>>>>>>> Note I don't have one of these so if anyone could test that would
>>>>>>>>>> be great!
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Jonathan Cameron <jic23@xxxxxxxxx>
>>>>>>>>>>
>>>>>>>>>> Jonathan Cameron (2):
>>>>>>>>>> Â staging:iio:adis16260 add id table support
>>>>>>>>>> Â staging:iio:adis16260 add suppport for adis16255 and adis16250.
>>>>>>>>>>
>>>>>>>>>> Âdrivers/staging/iio/gyro/Kconfig          |  Â8 +-
>>>>>>>>>> Âdrivers/staging/iio/gyro/adis16260.h        |  Â3 +
>>>>>>>>>> Âdrivers/staging/iio/gyro/adis16260_core.c     Â| Â139 ++++++++++++++------
>>>>>>>>>> Âdrivers/staging/iio/gyro/adis16260_platform_data.h | Â 19 +++
>>>>>>>>>> Âdrivers/staging/iio/gyro/gyro.h          Â|  Â9 ++
>>>>>>>>>> Â5 files changed, 136 insertions(+), 42 deletions(-)
>>>>>>>>>> Âcreate mode 100644 drivers/staging/iio/gyro/adis16260_platform_data.h
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>> Whilst I haven't tested these (don't have the hardware) I don't think there
>>>>>>>>> is anything controversial, so my intent is to push these to Greg before the
>>>>>>>>> next merge window. ÂThis is primarily to remove the duplication we currently
>>>>>>>>> have with two drivers that effectively cover the same parts.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Jonathan
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>>
>
>
--
motzblog.wordpress.com
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
