Driver sitronix CF1133 controller touchscreen #68321

clemdy · 2024-01-31T08:14:07Z

Add a new out of tree driver that support sitronix CF1133 controller touchscreen (I2C). Can be multitouch if needed.

github-actions · 2024-01-31T08:14:39Z

Hello @clemdy, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

drivers/input/input_cf1133.c

dts/bindings/input/sitronix,cf1133.yaml

drivers/input/CMakeLists.txt

drivers/input/Kconfig.cf1133

nordicjm · 2024-01-31T12:44:23Z

Seems your email is not setup in git properly, try:

git config --global user.name "Clement Dysli"
git config --global user.email "clementdy@kickmaker.net"
git commit --amend --reset-author
git push --force

clemdy · 2024-01-31T12:49:04Z

Yes my author name in the commit was bad i've corrected it

Queries addressed

fabiobaltieri

Hi, initial round of comment, mostly styling things, thanks!

dts/bindings/input/sitronix,cf1133.yaml

drivers/input/input_cf1133.c

fabiobaltieri · 2024-01-31T19:11:34Z

drivers/input/input_cf1133.c

+
+	gpio_init_callback(&(data->int_gpio_cb), cf1133_isr_handler, BIT(config->int_gpio.pin));
+
+	r = gpio_add_callback(config->int_gpio.port, &(data->int_gpio_cb));


Suggested change

r = gpio_add_callback(config->int_gpio.port, &(data->int_gpio_cb));

r = gpio_add_callback(config->int_gpio.port, &data->int_gpio_cb);

fabiobaltieri · 2024-01-31T19:13:07Z

drivers/input/Kconfig.cf1133

+
+if INPUT_CF1133
+
+config INPUT_CF1133_PERIOD


Suggested change

config INPUT_CF1133_PERIOD

config INPUT_CF1133_PERIOD_MS

drivers/input/Kconfig.cf1133

fabiobaltieri

Thanks, few more comments, also could you add this to https://github.com/zephyrproject-rtos/zephyr/blob/main/tests/drivers/build_all/input/app.overlay#L185 so it gets built in CI?

drivers/input/input_cf1133.c

clemdy · 2024-02-01T15:11:36Z

By the way, thank you very much Fabio for you advices and patience ;-)

fabiobaltieri · 2024-02-06T12:29:23Z

By the way, thank you very much Fabio for you advices and patience ;-)

Hey, you bet! Keep an eye out for compliance errors, since this is your first PR for the project we have to approve the run manually for every push but you can also run it locally using ./scripts/ci/check_compliance.py, saves you waiting for us to pick it up. After the first is PR is merged they'll just run automatically.

clemdy · 2024-02-06T15:51:27Z

By the way, thank you very much Fabio for you advices and patience ;-)

Hey, you bet! Keep an eye out for compliance errors, since this is your first PR for the project we have to approve the run manually for every push but you can also run it locally using ./scripts/ci/check_compliance.py, saves you waiting for us to pick it up. After the first is PR is merged they'll just run automatically.

Hey i think i have it, my tab size was 4 in my code editor instead of 8 by default for Gitlab that's why i have those issues. ^^

fabiobaltieri

Cool, one more round of comments, mostly styling related.

tests/drivers/build_all/input/app.overlay

drivers/input/input_cf1133.c

fabiobaltieri · 2024-02-27T17:28:57Z

@danieldegrasse can you do another review pass?

fabiobaltieri · 2024-03-06T10:17:52Z

@clemdy sorry this needs a rebase now, could you rebase and force push again please? You'll probably have to tweak the address in the i2c tests as well. Thanks for your patience.

fabiobaltieri · 2024-03-06T16:31:04Z

@clemdy sorry we don't take merge commits, just rebase it so there's a single commit on top of the current main.

faxe1008

Only found some stylistic inconsistencies. Good stuff 👍

faxe1008 · 2024-03-06T21:34:15Z

drivers/input/input_cf1133.c

+	return 0;
+}
+
+static int cf1133_get_protocol_type(struct cf1133_data *data, const struct cf1133_config *config)


Maybe its a preference thing, but there are some methods that take the deconstructed data and config and some take the entire device.

@clemdy I would recommend just passing dev to all internal functions and then get data and config internally, there's no performance penalties (the compiler optimizes all the pointer arithmetic out I think), you save a tiny bit of stack (only on pointer to pass) and you don't have to update the function signature if you need more stuff. I see at a bit like a self of Zephyr drivers.

drivers/input/input_cf1133.c

dts/bindings/input/sitronix,cf1133.yaml

bjarki-andreasen

Updating the Kconfig help field is important IMO as it can lead to a non trivial build error

drivers/input/Kconfig.cf1133

drivers/input/input_cf1133.c

fabiobaltieri · 2024-03-23T17:48:03Z

@clemdy there's a build error, plus I think one unaddressed stylistic comment (that I very much agree with). Thanks for your patience!

The driver allows to use CF1133 controller touchscreen (I2C) Signed-off-by: Clement Dysli <clementdy@kickmaker.net>

clemdy · 2024-03-25T13:27:25Z

Sorry @fabiobaltieri didn't want to re request your review i was aiming for @bjarki-trackunit (my page didn't refresh fast enough)

bjarki-andreasen

Looks good :)

github-actions · 2024-03-26T09:25:18Z

Hi @clemdy!
Congratulations on getting your very first Zephyr pull request merged 🎉🥳. This is a fantastic achievement, and we're thrilled to have you as part of our community!

To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge.

Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁

zephyrbot added area: Input Input Subsystem and Drivers area: Devicetree Binding PR modifies or adds a Device Tree binding labels Jan 31, 2024

zephyrbot requested review from fabiobaltieri, galak and gmarull January 31, 2024 08:14

zephyrbot assigned fabiobaltieri Jan 31, 2024

nordicjm previously requested changes Jan 31, 2024

View reviewed changes

drivers/input/input_cf1133.c Outdated Show resolved Hide resolved

dts/bindings/input/sitronix,cf1133.yaml Outdated Show resolved Hide resolved

drivers/input/CMakeLists.txt Outdated Show resolved Hide resolved

drivers/input/Kconfig.cf1133 Outdated Show resolved Hide resolved

clemdy force-pushed the main branch 3 times, most recently from 4a17f25 to 36b66aa Compare January 31, 2024 11:14

clemdy requested a review from nordicjm January 31, 2024 12:08

clemdy force-pushed the main branch from 36b66aa to 5db2057 Compare January 31, 2024 12:45

fabiobaltieri reviewed Jan 31, 2024

View reviewed changes

clemdy force-pushed the main branch from 5db2057 to c140ab7 Compare February 1, 2024 10:41

fabiobaltieri reviewed Feb 1, 2024

View reviewed changes

clemdy force-pushed the main branch from c140ab7 to bd132b2 Compare February 1, 2024 14:55

clemdy requested a review from fabiobaltieri February 5, 2024 08:16

clemdy force-pushed the main branch 3 times, most recently from e703433 to 46470c7 Compare February 6, 2024 12:12

clemdy force-pushed the main branch from 46470c7 to 1b4dde9 Compare February 6, 2024 15:50

fabiobaltieri reviewed Feb 6, 2024

View reviewed changes

clemdy force-pushed the main branch from 1b4dde9 to 2cde2ba Compare February 7, 2024 18:02

clemdy requested a review from danieldegrasse February 16, 2024 08:26

clemdy dismissed fabiobaltieri’s stale review via bc50b6d March 6, 2024 16:28

clemdy force-pushed the main branch 2 times, most recently from de33716 to acc443f Compare March 6, 2024 18:27

fabiobaltieri previously approved these changes Mar 6, 2024

View reviewed changes

fabiobaltieri requested review from faxe1008 and bjarki-andreasen March 6, 2024 18:51

faxe1008 previously approved these changes Mar 6, 2024

View reviewed changes

bjarki-andreasen requested changes Mar 7, 2024

View reviewed changes

drivers/input/Kconfig.cf1133 Show resolved Hide resolved

drivers/input/input_cf1133.c Outdated Show resolved Hide resolved

clemdy dismissed stale reviews from faxe1008 and fabiobaltieri via 07abc73 March 22, 2024 14:51

clemdy force-pushed the main branch 3 times, most recently from 87978a9 to 9d71a4d Compare March 22, 2024 16:09

clemdy force-pushed the main branch from 9d71a4d to b692975 Compare March 25, 2024 08:28

drivers: input: add cf1133 controller touchscreen

0d2aca2

The driver allows to use CF1133 controller touchscreen (I2C) Signed-off-by: Clement Dysli <clementdy@kickmaker.net>

clemdy force-pushed the main branch from b692975 to 0d2aca2 Compare March 25, 2024 10:18

fabiobaltieri approved these changes Mar 25, 2024

View reviewed changes

faxe1008 approved these changes Mar 25, 2024

View reviewed changes

clemdy requested review from bjarki-andreasen and fabiobaltieri March 25, 2024 13:25

clemdy requested a review from nordicjm March 26, 2024 08:36

bjarki-andreasen approved these changes Mar 26, 2024

View reviewed changes

fabiobaltieri merged commit bc140e1 into zephyrproject-rtos:main Mar 26, 2024
21 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Driver sitronix CF1133 controller touchscreen #68321

Driver sitronix CF1133 controller touchscreen #68321

clemdy commented Jan 31, 2024 •

edited by nordicjm

Loading

github-actions bot commented Jan 31, 2024

nordicjm commented Jan 31, 2024

clemdy commented Jan 31, 2024

fabiobaltieri left a comment

fabiobaltieri Jan 31, 2024

clemdy Feb 1, 2024

fabiobaltieri Jan 31, 2024

clemdy Feb 1, 2024

fabiobaltieri left a comment

clemdy commented Feb 1, 2024

fabiobaltieri commented Feb 6, 2024

clemdy commented Feb 6, 2024

fabiobaltieri left a comment

fabiobaltieri commented Feb 27, 2024

fabiobaltieri commented Mar 6, 2024 •

edited

Loading

fabiobaltieri commented Mar 6, 2024

faxe1008 left a comment

faxe1008 Mar 6, 2024

fabiobaltieri Mar 23, 2024 •

edited

Loading

bjarki-andreasen left a comment

fabiobaltieri commented Mar 23, 2024

clemdy commented Mar 25, 2024

bjarki-andreasen left a comment

github-actions bot commented Mar 26, 2024


		gpio_init_callback(&(data->int_gpio_cb), cf1133_isr_handler, BIT(config->int_gpio.pin));

		r = gpio_add_callback(config->int_gpio.port, &(data->int_gpio_cb));

Driver sitronix CF1133 controller touchscreen #68321

Driver sitronix CF1133 controller touchscreen #68321

Conversation

clemdy commented Jan 31, 2024 • edited by nordicjm Loading

github-actions bot commented Jan 31, 2024

nordicjm commented Jan 31, 2024

clemdy commented Jan 31, 2024

fabiobaltieri left a comment

Choose a reason for hiding this comment

fabiobaltieri Jan 31, 2024

Choose a reason for hiding this comment

clemdy Feb 1, 2024

Choose a reason for hiding this comment

fabiobaltieri Jan 31, 2024

Choose a reason for hiding this comment

clemdy Feb 1, 2024

Choose a reason for hiding this comment

fabiobaltieri left a comment

Choose a reason for hiding this comment

clemdy commented Feb 1, 2024

fabiobaltieri commented Feb 6, 2024

clemdy commented Feb 6, 2024

fabiobaltieri left a comment

Choose a reason for hiding this comment

fabiobaltieri commented Feb 27, 2024

fabiobaltieri commented Mar 6, 2024 • edited Loading

fabiobaltieri commented Mar 6, 2024

faxe1008 left a comment

Choose a reason for hiding this comment

faxe1008 Mar 6, 2024

Choose a reason for hiding this comment

fabiobaltieri Mar 23, 2024 • edited Loading

Choose a reason for hiding this comment

bjarki-andreasen left a comment

Choose a reason for hiding this comment

fabiobaltieri commented Mar 23, 2024

clemdy commented Mar 25, 2024

bjarki-andreasen left a comment

Choose a reason for hiding this comment

github-actions bot commented Mar 26, 2024

clemdy commented Jan 31, 2024 •

edited by nordicjm

Loading

fabiobaltieri commented Mar 6, 2024 •

edited

Loading

fabiobaltieri Mar 23, 2024 •

edited

Loading