Size: 6731
Comment:
|
Size: 8395
Comment: Add mlock coding task
|
Deletions are marked like this. | Additions are marked like this. |
Line 3: | Line 3: |
These tasks are for you to get familiar with the IIO subsystem. You do not need to claim these via the Outreachy tasks page. For IIO related questions you can join #linux-iio IRC channel (server irc.oftc.net). | These tasks are for you to get familiar with the IIO subsystem. You do not need to claim these via the Outreachy tasks page. Please post all questions to outreachy-kernel@googlegroups.com. IIO mentors and other applicants will respond on the list and it will serve as a troubleshooting resource for all applicants. You can also join #linux-iio IRC channel (server irc.oftc.net) to ask questions of the IIO community. |
Line 107: | Line 111: |
== Task 1: Replace IIO_DEV_ATTR_* with IIO_CHAN_INFO_* == | == Coding Task 1: == |
Line 109: | Line 113: |
As the IIO subsystem evolves, attributes that were once privately defined become standard and hence a special global define is used. This kind of evolution means we can update existing drivers to use the new defines. | Clean-up idea: IIO Headers: This was previously posted to the outreachy-kernel@googlegroups.com See description and follow-ups here: https://groups.google.com/forum/#!topic/outreachy-kernel/LtI90_SwjHE I believe this task is completed or close to being completed. If you find a driver which you think applies here, be sure to serach for it in the outreachy group mailing list to see if it has already been done or disregarded. == Coding Task 2: == Coding Task 2: Replace driver usages of mlock with a private lock ===== Background: ===== The locking scheme in the IIO subsystem includes a lock in the iio_dev structure called mlock. The usage of iio_dev->mlock is being redefined as protecting operating mode changes - as in changes between BUFFER* and DIRECT modes. Notice how the struct iio_dev fields are labelled [INTERN] or [DRIVER]. The mlock will revert to an [INTERN] field once all the non-conforming usages are removed from drivers. The IIO core functions in (drivers/iio/industrialio-*.c) will be the only users of mlock. Drivers will use helper functions to control operating mode changes. That's a bunch of background that you don't need to totally grasp to do this task, but it's nice to see the bigger picture. The piece of the migration this task focuses on is removing the usages of mlock that don't meet the new model. I suspect we'll find that these drivers were using mlock as it was previously defined: "to protect simultaneous device *state* changes." Typically this means they are changing some configuration bits in the hardware. Those changes are important to protect, just do it with a driver private lock, not mlock. ===== PATCH Example: ===== |
Line 112: | Line 148: |
commit e0f3fc9b47e6 iio: accel: sca3000_core: implemented IIO_CHAN_INFO_SAMP_FREQ |
staging: iio: ad9832: replace mlock with driver private lock https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/?h=testing&id=388c8f18ff29fe95dbf72cb0a1bd8fbcd6f52f8f |
Line 115: | Line 151: |
http://git.kernel.org/cgit/linux/kernel/git/jic23/iio.git/commit/?h=testing&id=e0f3fc9b47e61bb5d79f2bb8684d80eee5aaac99 | This is probably the simplest case patch for this task. I want to emphasize simple, because I did pass over a few others on my way to creating an example patch. I saved the more interesting drivers for you ;) |
Line 117: | Line 155: |
You may uncover issues with the usage or placement of locks in general. | |
Line 118: | Line 157: |
Note that aside from looking at the patch itself, you can find the email chatter about it in the linux-iio mailing list. | Your reviewer may notice other things that need tidying up. If they're not immediately required for this patch, save it for a follow up patch. |
Line 120: | Line 160: |
Find a driver in the IIO staging directory that has one or more IIO_DEV_ATTR* that can be updated to IIO_CHAN_INFO* attributes. | ===== The Directions: ===== |
Line 122: | Line 162: |
UPDATE: No instances remain in drivers/staging. If you have not created a patch for this yet, please look in drivers/iio. Note: * You'll get a basic understanding of how the attributes are defined and used while doing this work. That'll take some exploring. * The example patch I referenced startled me at first with the diffstat! You'll find that it's movement of code, not so much creation of new code that led to so many changes. |
Your mission is to remove the usage of mlock from the IIO drivers in staging. A simple 'git grep mlock' finds the drivers and I've posted that list on the Outreachy tasks page. |
Line 128: | Line 166: |
== Task 2: Use sparse for endianess verification == | Please sign up for one driver at a time, take it all the way through ACK, before selecting another one. Be sure to follow along as these are posted and learn from others patches. |
Line 130: | Line 170: |
With this task you will learn about the endianness conversions. | ===== PATCHES need to be sent to all of: ===== |
Line 132: | Line 172: |
See article here https://lwn.net/Articles/205624/ and here: https://kernelnewbies.org/EndianIssues | outreachy-kernel@googlegroups.com, |
Line 134: | Line 174: |
Use the following commands to check for endianness warnings in IIO code: * {{{make C=2 CF=-D__CHECK_ENDIAN__ drivers/iio/}}} * {{{make C=2 CF=-D__CHECK_ENDIAN__ drivers/staging/iio/}}} |
the IIO Maintainer, IIO Reviewers, and IIO Mailing List, |
Line 138: | Line 176: |
In order to get relevant results from sparse on IIO you need to select for building the corresponding modules. |
Jonathan Cameron <jic23@kernel.org> (maintainer:IIO SUBSYSTEM AND DRIVERS) Hartmut Knaack <knaack.h@gmx.de> (reviewer:IIO SUBSYSTEM AND DRIVERS) Lars-Peter Clausen <lars@metafoo.de> (reviewer:IIO SUBSYSTEM AND DRIVERS) Peter Meerwald <pmeerw@pmeerw.net> (reviewer:IIO SUBSYSTEM AND DRIVERS) linux-iio@vger.kernel.org (open list:IIO SUBSYSTEM AND DRIVERS) |
Line 141: | Line 182: |
You either run make menuconfig and manually go to Drivers -> IIO and select each driver with Y or M for build. |
and, for good measure, Jonathan likes to make sure the original Author(s) are directly copied on all patches. So, grab the MODULE_AUTHOR found at the tail end of the driver source. |
Line 144: | Line 186: |
Or run make allyesconfig before running sparse command. UPDATE: All instances in IIO have been patched or claimed. If you're still interested in the topic, look elsewhere in staging drivers. Those would not need to be claimed as they are not IIO tasks. |
Post questions to outreachy-kernel@googlegroups.com or the #linux-iio IRC channel (server irc.oftc.net). |
These tasks are for you to get familiar with the IIO subsystem. You do not need to claim these via the Outreachy tasks page.
Please post all questions to outreachy-kernel@googlegroups.com. IIO mentors and other applicants will respond on the list and it will serve as a troubleshooting resource for all applicants.
You can also join #linux-iio IRC channel (server irc.oftc.net) to ask questions of the IIO community.
Please email your solutions to amsfield22 at gmail dot com and daniel.baluta at gmail dot com. Your email should have the subject Task XX: Short task description
Experimenting with IIO subsystem
For this we will use two kernel modules found in drivers/iio/dummy:
iio_dummy_evgen.ko - generates fake events interrupts to be used by the iio_dummy example driver
implementation for this module is in iio_dummy_evgen.c
iio_dummy.ko - example IIO driver to demonstrate existing functionality
core implementation can be found in iio_simple_dummy.c
buffer functionality is implemented in iio_simple_dummy_buffer.c
events functionality is implemented in iio_simple_dummy_events.c
Dummy modules compilation
You need to select the following config options:
CONFIG_IIO_DUMMY_EVGEN - for building iio_dummy_evgen kernel module
CONFIG_IIO_SIMPLE_DUMMY - for building iio_dummy kernel module
CONFIG_IIO_SIMPLE_DUMMY_EVENTS, CONFIG_IIO_SIMPLE_DUMMY_BUFFER should be selected for events and buffer functionality.
CONFIG_IIO_CONFIGFS - for creating the dummy device under configfs
- Mount the configfs filesystem: read Documentation/iio/iio_configfs.txt
Use the following commands for modules compilation:
$ make drivers/iio/dummy/iio_dummy_evgen.ko
$ make drivers/iio/dummy/iio_dummy.ko
Use the following commands for module loading:
$ insmod iio_dummy_evgen.ko
$ insmod iio_dummy.ko
Use the following command to create your dummy device under the configfs filesystem:
$ mkdir /config/iio/devices/dummy/my_dummy_device
Task 01:
- Show that the modules were successfully loaded.
- lsmod | grep dummy
- ls -l /config/iio/devices/dummy/
- ls -l /sys/bus/iio/devices/iio:device0/
- ls -l /sys/bus/iio/devices/iio_evgen/
Task 02:
- Add channels for a 3-axis compass to iio_simple_dummy module.
- Show that channels were successfully added
- ls -l /sys/bus/iio/devices/iio:device0/
- ls -l /sys/bus/iio/devices/iio:device0/scan_elements
- create a patch with your changes
Hints:
- channel type for a compass is IIO_MAGN
- users should be able to read raw readings from each axis
- users should be able to read a shared scale
- users should be able to access data via a buffer
- data is unsigned, resolution is 16 bits, storage is 16 bits
- compass doesn't support events
IIO event monitor
IIO event monitor is an user space example application which reads events from IIO layer and pretty prints the results. Implementation can be found in iio_event_monitor.c under tools/iio/. BR
Task 03:
Compile iio_event_monitor:
compile iio_event_monitor.c to obtain an executable called iio_event_monitor.
- send us the command(s) used to successfully compile iio_event_monitor.c
Task 04:
Read events using iio_event_monitor
- run iio_event_monitor without arguments to figure out how it should be used
- read events from iio_dummy module
Hints
- Events are generated by iio_dummy_evgen via sysfs (look inside /sys/bus/iio/devices/iio_evgen/)
- send us the commands used to read/generate the events
Generic buffer
Task 05:
Create triggers using configfs interface.
- read Documentation/iio/iio_configfs.txt in order to create a software trigger named t1.
- where in the sysfs hierarchy does the trigger resides?
- sends us the commands used to create the trigger
Task 06:
Read samples from buffer generated by the iio_dummy module.
- compile iio_generic_buffer.c from tools/iio. This program will be used to read data from buffer.
have a look at ./iio_generic_buffer -h options. You will use the trigger t1 created with the previous tasks and iio_dummy_part_no for IIO device.
- send us the full command history
Coding Tasks
Note for all IIO Coding Tasks:
- Please use the task page to claim the driver you are going to patch.
Please 'CC outreachy-kernel@googlegroups.com on all questions related to this so everyone can benefit.
Coding Task 1:
Clean-up idea: IIO Headers: This was previously posted to the outreachy-kernel@googlegroups.com
See description and follow-ups here: https://groups.google.com/forum/#!topic/outreachy-kernel/LtI90_SwjHE
I believe this task is completed or close to being completed. If you find a driver which you think applies here, be sure to serach for it in the outreachy group mailing list to see if it has already been done or disregarded.
Coding Task 2:
Coding Task 2: Replace driver usages of mlock with a private lock
Background:
The locking scheme in the IIO subsystem includes a lock in the iio_dev structure called mlock. The usage of iio_dev->mlock is being redefined as protecting operating mode changes - as in changes between BUFFER* and DIRECT modes.
Notice how the struct iio_dev fields are labelled [INTERN] or [DRIVER]. The mlock will revert to an [INTERN] field once all the non-conforming usages are removed from drivers. The IIO core functions in (drivers/iio/industrialio-*.c) will be the only users of mlock. Drivers will use helper functions to control operating mode changes.
That's a bunch of background that you don't need to totally grasp to do this task, but it's nice to see the bigger picture.
The piece of the migration this task focuses on is removing the usages of mlock that don't meet the new model. I suspect we'll find that these drivers were using mlock as it was previously defined: "to protect simultaneous device *state* changes." Typically this means they are changing some configuration bits in the hardware. Those changes are important to protect, just do it with a driver private lock, not mlock.
PATCH Example:
Review this recently submitted patch: staging: iio: ad9832: replace mlock with driver private lock https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/?h=testing&id=388c8f18ff29fe95dbf72cb0a1bd8fbcd6f52f8f
This is probably the simplest case patch for this task. I want to emphasize simple, because I did pass over a few others on my way to creating an example patch. I saved the more interesting drivers for you
You may uncover issues with the usage or placement of locks in general.
Your reviewer may notice other things that need tidying up. If they're not immediately required for this patch, save it for a follow up patch.
The Directions:
Your mission is to remove the usage of mlock from the IIO drivers in staging. A simple 'git grep mlock' finds the drivers and I've posted that list on the Outreachy tasks page.
Please sign up for one driver at a time, take it all the way through ACK, before selecting another one. Be sure to follow along as these are posted and learn from others patches.
PATCHES need to be sent to all of:
outreachy-kernel@googlegroups.com,
the IIO Maintainer, IIO Reviewers, and IIO Mailing List,
Jonathan Cameron <jic23@kernel.org> (maintainer:IIO SUBSYSTEM AND DRIVERS) Hartmut Knaack <knaack.h@gmx.de> (reviewer:IIO SUBSYSTEM AND DRIVERS) Lars-Peter Clausen <lars@metafoo.de> (reviewer:IIO SUBSYSTEM AND DRIVERS) Peter Meerwald <pmeerw@pmeerw.net> (reviewer:IIO SUBSYSTEM AND DRIVERS) linux-iio@vger.kernel.org (open list:IIO SUBSYSTEM AND DRIVERS)
and, for good measure, Jonathan likes to make sure the original Author(s) are directly copied on all patches. So, grab the MODULE_AUTHOR found at the tail end of the driver source.
Post questions to outreachy-kernel@googlegroups.com or the #linux-iio IRC channel (server irc.oftc.net).