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

GSoC Warm Up Kernel Task #234

Open
wants to merge 17 commits into
base: 5.4-rt
Choose a base branch
from
Open

GSoC Warm Up Kernel Task #234

wants to merge 17 commits into from

Conversation

NiklasWan
Copy link

This is the pull request for Media Ip Streaming project, which implements a dummy kernel driver for the GSoC 2020 warm up task.

RobertCNelson and others added 17 commits March 10, 2020 13:24
hartkopp/can-isotp@ced84ca
Signed-off-by: Robert Nelson <robertcnelson@gmail.com>
Signed-off-by: Robert Nelson <robertcnelson@gmail.com>
patch-5.4.19-rt11.patch.xz

Signed-off-by: Robert Nelson <robertcnelson@gmail.com>
Reference: v5.6-rc5
Signed-off-by: Robert Nelson <robertcnelson@gmail.com>
Reference: v5.5.8
Signed-off-by: Robert Nelson <robertcnelson@gmail.com>
Signed-off-by: Robert Nelson <robertcnelson@gmail.com>
Signed-off-by: Robert Nelson <robertcnelson@gmail.com>
Copy link
Contributor

@dlech dlech left a comment

Choose a reason for hiding this comment

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

Good luck with your project. Looks like you are off to a good start.

I just wanted to share a few housekeeping things I learned from contributing to the Linux kernel (see inline comments).

config GSOC_DUMMY_CHAR_DEVICE
tristate "GSoC Warmup Driver"
help
This builds the GSoC Dummy Char Driver, which is required for the warmup task.
Copy link
Contributor

Choose a reason for hiding this comment

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

Help lines should be indented with 1 tab + 2 spaces

only mixes the entropy pool.
Copy link
Contributor

Choose a reason for hiding this comment

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

Watch out for unintentional changes like editors that automatically add a newline at the end of the file.

* Dummy character driver by
* John Madieu <john.madieu@gmail.com>
*
* This program is free software; you can redistribute it and/or
Copy link
Contributor

Choose a reason for hiding this comment

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

Using SPDX license identifier is used nowdays instead of license paragraphs.

https://www.kernel.org/doc/html/latest/process/license-rules.html

#include <linux/cdev.h>
#include <linux/uaccess.h>

static int gsoc_char_dev_open(struct inode * inode, struct file * file);
Copy link
Contributor

Choose a reason for hiding this comment

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

It probably depends on the area of the kernel but most code I have seen tries to avoid forward declarations if possible by rearranging the code.

// register a range of char dev numbers 0 to 1 in this case
err = alloc_chrdev_region(&gsoc_dev, 0, 1, "gsoc_dummy_char_dev");
if (err < 0) {
pr_err("Can't get major number\n");
Copy link
Contributor

Choose a reason for hiding this comment

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

It is a good idea to include the module name in the error message so users know which module the error came from.

static ssize_t gsoc_char_dev_read(struct file *file, char __user * buf, size_t count,
loff_t * offset)
{
pr_info("Read function of GSoC dummy driver called.\n");
Copy link
Contributor

Choose a reason for hiding this comment

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

It could be fun to actually put something in the buffer so that you can actually read something from the character device.

@NiklasWan
Copy link
Author

Hey David,

thank you very much for these helpful tricks.

Best,
Niklas

@RobertCNelson RobertCNelson force-pushed the 5.4-rt branch 3 times, most recently from a0eb9c6 to f1d27f8 Compare May 21, 2021 18:00
@RobertCNelson RobertCNelson force-pushed the 5.4-rt branch 2 times, most recently from 1296885 to 1ec7015 Compare June 3, 2021 01:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants