Skip to content

drivers/lis3dh: fixed bug and simplified init()#6479

Merged
jnohlgard merged 1 commit intoRIOT-OS:masterfrom
haukepetersen:fix_lis3dh_mulle
Jan 31, 2017
Merged

drivers/lis3dh: fixed bug and simplified init()#6479
jnohlgard merged 1 commit intoRIOT-OS:masterfrom
haukepetersen:fix_lis3dh_mulle

Conversation

@haukepetersen
Copy link
Copy Markdown
Contributor

@haukepetersen haukepetersen commented Jan 26, 2017

Now the driver is working fine here at my desk with the mulle board.

One thing that I find a little confusing, though: The driver is running the SPI bus in MODE_0 (pol and pha set to 0). From the datasheet, the device should run however in MODE_3 (pol and pha set to 1). But when I tell the driver to use MODE_3, I can't interact with the sensor anymore...

@gebart: would you mind to have a quick look at this? Thx!

@haukepetersen
drivers/lis3dh: fixed bug and simplified init()
ed2bb9d 
- changed to SPI_MODE_0
- made init() function use the params struct as parameter
@haukepetersen haukepetersen added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: drivers Area: Device drivers labels Jan 26, 2017
@haukepetersen haukepetersen added this to the Release 2017.01 milestone Jan 26, 2017
@PeterKietzmann PeterKietzmann mentioned this pull request Jan 26, 2017
Closed
23 tasks
@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Jan 26, 2017

I don't have the device to test this. Blind ACK makes no sense. It is a 'known' leftover of the SPI rework noted in #6437. @gebart please test this on mulle. We can backport bugfixes later

@haukepetersen
Copy link
Copy Markdown
Contributor Author

haukepetersen commented Jan 26, 2017

@kYc0o: or could you give this a quick test run?!

@PeterKietzmann PeterKietzmann added the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Jan 26, 2017
@jnohlgard
Copy link
Copy Markdown
Member

jnohlgard commented Jan 27, 2017

@haukepetersen Nice! Thanks for cleaning up the driver. I'll give it a spin right now.

@jnohlgard
Copy link
Copy Markdown
Member

jnohlgard commented Jan 27, 2017 •
edited
Loading

I agree that from the data sheet it looks like it should be using mode 3. In Contiki, the configuration we have is using mode 3 for the device.
I don't have an oscilloscope or logic analyzer available right now so I can't examine the signals, but I wonder if there might be some delays in the hardware state when we reconfigure the CTAR register in spi_acquire before the outputs go to their idle state?
The Contiki implementation uses CTAR0 for SPI mode 0 transfers and CTAR1 for mode 3 transfers, so it uses the hardware a bit differently compared to this implementation.
edit: And with the old RIOT SPI implementation the driver was also using mode 3 (SECOND_FALLING)

jnohlgard
jnohlgard approved these changes Jan 27, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@jnohlgard jnohlgard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The driver works for me, the SPI mode handling needs further investigation on Kinetis.

@haukepetersen haukepetersen added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 27, 2017
@haukepetersen
Copy link
Copy Markdown
Contributor Author

haukepetersen commented Jan 27, 2017

I will look at the SPI output on my logic analyzer later today, to make sure the kinetis is actually doing what it promises to do...

@jnohlgard
Copy link
Copy Markdown
Member

jnohlgard commented Jan 27, 2017

@haukepetersen thanks!

@cgundogan
Copy link
Copy Markdown
Member

cgundogan commented Jan 27, 2017

FYI: Jenkins fails because this PR is not rebased to current master, where the emb6 with msb-430 issue is solved. No need to rebase though, Jenkins can be ignored here.

@cgundogan
Copy link
Copy Markdown
Member

cgundogan commented Jan 27, 2017

nvm, Jenkins is green now. Did not refresh the website ..

@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Jan 31, 2017

@gebart would you mind to merge this PR?

@jnohlgard
Copy link
Copy Markdown
Member

jnohlgard commented Jan 31, 2017

It seems fine. I would like to see the SPI mode fixed though, but that's likely a periph bug and should go in its own PR.

@jnohlgard jnohlgard merged commit 9a3589d into RIOT-OS:master Jan 31, 2017
@haukepetersen haukepetersen deleted the fix_lis3dh_mulle branch February 8, 2017 10:12
@miri64
Copy link
Copy Markdown
Member

miri64 commented Oct 18, 2017

There is a new release pending so at this point the backporting-needed label is only noise ;-)

@miri64 miri64 removed the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Oct 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jnohlgard jnohlgard jnohlgard approved these changes

Assignees

@jnohlgard jnohlgard

@PeterKietzmann PeterKietzmann

Labels

Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@haukepetersen @PeterKietzmann @jnohlgard @cgundogan @miri64