Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mpu6050 impr3 add cdev #31

Open
wants to merge 3 commits into
base: Dmytro.Kirtoka
Choose a base branch
from

Conversation

dmytrokirtoka
Copy link

@dmytrokirtoka dmytrokirtoka commented Nov 15, 2017

complete mpu6050 dev for next use.

  • added sysfs ops for debug
  • added buffered read from device
  • added read by time with timer and work
  • added support read fops for read all data at once

@dmytrokirtoka
Copy link
Author

finished the driver, ready for review

@dmytrokirtoka
Copy link
Author

three dependent commits for mpu6050 practics:

  1. compact driver
  2. add buffering, add timer
  3. add read for next use and check with dd if=/dev/mpu6050 of=/my_file bs=7*4 count 1.

@dmytrokirtoka
Copy link
Author

Driver change:
added 10 element buffer
added two module param:
rate for timer rate (default 1000 msec)
is_buf for buffered/not buffered read
added sync by complete
safe ring buffer positioning with spinlock

@dmytrokirtoka
Copy link
Author

I'm tested last changes and work at fix it

added ring buffer with 10 elm
added timer with module parameter rate, default is 1000 msec
added work for periodic read data from device sheduled by time
added dinamic add/remove device in /dev

added support read only fops for mpu6050 module

limitation:
  read data by only READ_REG_NUM blocks

  supported only one device
@dmytrokirtoka
Copy link
Author

Check on device, apply kernel code style.

Signed-off-by: dmytro.kirtoka <[email protected]>
@an1kh
Copy link
Contributor

an1kh commented Dec 12, 2017

@dmytrokirtoka , is this PR is still relevant?
#46 looks to contain these changes and can be merged.

@dmytrokirtoka
Copy link
Author

is this PR is still relevant?

@dmytrokirtoka
Copy link
Author

this pull request have another realization of buffering, see branch "added buffered read data by time",
have char device implementation and work task.

Copy link
Collaborator

@AleksandrBulyshchenko AleksandrBulyshchenko left a comment

Choose a reason for hiding this comment

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

👍
I haven't tested - only reviewed.
Good exercise implementation (timer, completion, etc).

Nitpicks:

  • Commit messages could be improved (following kernel recommendations, SOB, etc.)
  • Changes of "mpu6050: apply kernel code style" which fixes previous commits are better to include in place.


return 0;
}

static int mpu6050_get_data(int *values, int is_buffered)
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO, there's no need to make is_buffered a parameter here.
If you check is_buf inside mpu6050_read_data(), I believe mpu6050_get_data() should follow the same.

g_mpu6050_data.data[head].values.temperature = (temp + 12420 + 170) / 340;

if ((is_buf && count < READ_DEPTH) || (!is_buf && !count))
complete(&has_data);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just to note: I haven't made deep analyse bu afraid of possibility of some race with mpu6050_get_data(),
which may cause has_data became desynchronized with g_mpu6050_data.count.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants