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

CHSC6X driver #71073

Merged
merged 1 commit into from Apr 8, 2024
Merged

CHSC6X driver #71073

merged 1 commit into from Apr 8, 2024

Conversation

nicogou
Copy link
Contributor

@nicogou nicogou commented Apr 3, 2024

Added a driver for the CHSC6X chip used on the Seeed Studio Round Display for Xiao

@zephyrbot zephyrbot added area: Devicetree Binding PR modifies or adds a Device Tree binding area: Input Input Subsystem and Drivers labels Apr 3, 2024
@nicogou
Copy link
Contributor Author

nicogou commented Apr 3, 2024

Hi, I opened this PR in order to add the driver for the touch controller on the Seeed Studio Round display ( https://wiki.seeedstudio.com/get_start_round_display/ ), but my final goal is to add the display as a shield in Zephyr.
Should I wait for this PR to be merged and then open another one where I add the shield, or can I add the shield in this one ?
Thanks!
(Sorry if the question is dumb)

@fabiobaltieri
Copy link
Member

fabiobaltieri commented Apr 4, 2024

Hi, I opened this PR in order to add the driver for the touch controller on the Seeed Studio Round display ( https://wiki.seeedstudio.com/get_start_round_display/ ), but my final goal is to add the display as a shield in Zephyr. Should I wait for this PR to be merged and then open another one where I add the shield, or can I add the shield in this one ? Thanks!

Separate PR is ideal, you can either wait or open the shield one already and just point out that this has to go in first, but if you are not in a hurry just do one at a time.

@fabiobaltieri
Copy link
Member

@nicogou you have few compliance errors and a missing copyright line, you can also run compliance locally with ./scripts/ci/check_compliance.py

@nicogou nicogou force-pushed the chsc6x-driver branch 2 times, most recently from b476daf to cad2bfb Compare April 4, 2024 09:15
@nicogou
Copy link
Contributor Author

nicogou commented Apr 4, 2024

@nicogou you have few compliance errors and a missing copyright line, you can also run compliance locally with ./scripts/ci/check_compliance.py

I made some changes that should resolve these.
I'll open a separate PR for the shield when I'm ready.

Copy link
Member

@fabiobaltieri fabiobaltieri left a comment

Choose a reason for hiding this comment

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

could you also add an entry for this in tests/drivers/build_all/input/app.overlay? thanks

drivers/input/input_chsc6x.c Outdated Show resolved Hide resolved
drivers/input/input_chsc6x.c Outdated Show resolved Hide resolved
drivers/input/input_chsc6x.c Outdated Show resolved Hide resolved
drivers/input/input_chsc6x.c Outdated Show resolved Hide resolved
dts/bindings/input/seeed,chsc6x.yaml Outdated Show resolved Hide resolved
@nicogou nicogou force-pushed the chsc6x-driver branch 2 times, most recently from d25be07 to 32e1272 Compare April 4, 2024 11:31
@nicogou
Copy link
Contributor Author

nicogou commented Apr 4, 2024

@fabiobaltieri Changes made, tested and pushed

drivers/input/input_chsc6x.c Outdated Show resolved Hide resolved
dts/bindings/input/seeed,chsc6x.yaml Outdated Show resolved Hide resolved
@nicogou nicogou force-pushed the chsc6x-driver branch 2 times, most recently from 7566696 to 191bab0 Compare April 4, 2024 13:10
fabiobaltieri
fabiobaltieri previously approved these changes Apr 4, 2024
Comment on lines 5 to 11
bool "Use out of tree CHSC6X capacitive touch panel driver"
default y
depends on DT_HAS_CHIPSEMI_CHSC6X_ENABLED
select GPIO
select I2C
help
Enable out of tree driver for CHSC6X touch panel.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
bool "Use out of tree CHSC6X capacitive touch panel driver"
default y
depends on DT_HAS_CHIPSEMI_CHSC6X_ENABLED
select GPIO
select I2C
help
Enable out of tree driver for CHSC6X touch panel.
bool "Use CHSC6X capacitive touch panel driver"
default y
depends on DT_HAS_CHIPSEMI_CHSC6X_ENABLED
select GPIO
select I2C
help
Enable driver for CHSC6X touch panel.

Comment on lines 54 to 58
/* These events are generated for the LVGL touch implementation. */
input_report_abs(dev, INPUT_ABS_X, col, false, K_FOREVER);
input_report_abs(dev, INPUT_ABS_Y, row, false, K_FOREVER);
input_report_key(dev, INPUT_BTN_TOUCH, 1, true, K_FOREVER);
} else {
/* This event is generated for the LVGL touch implementation. */
input_report_key(dev, INPUT_BTN_TOUCH, 0, true, K_FOREVER);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure I understand the mentions to LVGL?

Comment on lines 12 to 14
The irq signal defaults to active low as produced by the
sensor. The property value should ensure the flags properly
describe the signal that is presented to the driver.
Copy link
Collaborator

Choose a reason for hiding this comment

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

@@ -0,0 +1,14 @@
description: CHSC6X touchscreen sensor
Copy link
Collaborator

Choose a reason for hiding this comment

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

File has the wrong vendor in its name

@nicogou
Copy link
Contributor Author

nicogou commented Apr 5, 2024

Made the requested changes

@kartben
Copy link
Collaborator

kartben commented Apr 5, 2024

Made the requested changes

You might not have pushed them correctly, though :)

@zephyrbot
Copy link
Collaborator

zephyrbot commented Apr 5, 2024

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff

Note: This message is automatically posted and updated by the Manifest GitHub Action.

Added a driver for the CHSC6X chip used on the
Seeed Studio Round Display for Xiao

Signed-off-by: Nicolas Goualard <nicolas.goualard@sfr.fr>
@nicogou
Copy link
Contributor Author

nicogou commented Apr 8, 2024

Made the requested changes

You might not have pushed them correctly, though :)

Indeed, just redid it, sorry

@jukkar jukkar removed their request for review April 8, 2024 10:23
@nashif nashif merged commit d9fd292 into zephyrproject-rtos:main Apr 8, 2024
23 checks passed
Copy link

github-actions bot commented Apr 8, 2024

Hi @nicogou!
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! 🪁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Devicetree Binding PR modifies or adds a Device Tree binding area: Input Input Subsystem and Drivers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants