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

add additional overrides to rotary-encoder overlay #2334

Merged
merged 2 commits into from
Jan 5, 2018

Conversation

shawaj
Copy link

@shawaj shawaj commented Jan 5, 2018

Updates to the rotary-encoder overlay to add additional overrides that are possible according to this doc - https://elixir.free-electrons.com/linux/latest/source/Documentation/devicetree/bindings/input/rotary-encoder.txt

See #2331 for more details also

@pelwell
Copy link
Contributor

pelwell commented Jan 5, 2018

I've removed the trailing whitespace, but now your overlay seems to be knobless.

@shawaj
Copy link
Author

shawaj commented Jan 5, 2018

Darn! I guess I should change "knob" to "rotary0"?

@pelwell
Copy link
Contributor

pelwell commented Jan 5, 2018

Or vice versa - as long as everything agrees it doesn't matter.

@pelwell
Copy link
Contributor

pelwell commented Jan 5, 2018

You also need to think about the types of the properties you are applying the overrides to - currently all the new parameters are declared to target strings.

@shawaj shawaj force-pushed the rpi-4.9.y branch 2 times, most recently from 2e6f0b4 to 47471dc Compare January 5, 2018 10:55
@shawaj
Copy link
Author

shawaj commented Jan 5, 2018

@pelwell ok fixed that issue.

about the other thing you have mentioned - i am not entirely sure what you mean. do i need to set the default values up in fragment@1?

@pelwell
Copy link
Contributor

pelwell commented Jan 5, 2018

Although Device Tree contains no real type information - once compiled it is all just arrays of bytes - properties can be written as strings or arrays of integers of various sizes. Device drivers reading the properties then place an interpretation on those arrays of bytes, usually one that matches the way the property was written in the first place but not necessarily so.

The DT parameter documentation describes the various parameter types and how to declare them. The new properties are mainly integers, but there is a boolean property which is true if it exists and false if it doesn't.

@shawaj
Copy link
Author

shawaj commented Jan 5, 2018

i guess linux,axis, steps, steps-per-period need to have :0 after them as they would be numbers?

For rotary-encoder,relative-axis - that one doesn't appear to have anything with it? It seems as if the driver "defaults" to an absolute axis but then changes to relative if you declare it? So should I remove that from the fragment@0 part and only declare it in overrides so you can use either?

and there is the wakeup-source one which is boolean - does that work the same that if declared it is true and if not declared anywhere it is false? The rotary-encoder,rollover one seems to be the same also

@pelwell
Copy link
Contributor

pelwell commented Jan 5, 2018

rotary-encoder,relative-axis, rotary-encoder,rollover and wakeup-source are booleans, which you declare with a ? at the end. You create a boolean property which defaults to true by just writing the name without the assignment in the overlay - wakeup-source;. If it defaults to false - as all of these probably should - then omit the property from the fragment.

I think it is good practice to put the default property values explicitly into the fragments, for ease of understanding and to protect against a change in the driver's defaults - except for false booleans...

@shawaj
Copy link
Author

shawaj commented Jan 5, 2018

is there a way to check if the booleans default to false? it seems from the txt file I linked to above that they are all false, but I just wondered for future?

@pelwell
Copy link
Contributor

pelwell commented Jan 5, 2018

I was going to say that there's no easy way, and that I'd read the source (which I had), but it's obvious that booleans have to default (in the driver at least) to false, otherwise you can't distinguish an explicit override from leaving it at the default. What we choose to make the overlay default value is up to us.

@shawaj
Copy link
Author

shawaj commented Jan 5, 2018

ok updated - how does that look?

@shawaj
Copy link
Author

shawaj commented Jan 5, 2018

Hold on - spotted some errors already....one minute and I will fix!!

@shawaj
Copy link
Author

shawaj commented Jan 5, 2018

ok - take a look now

@pelwell
Copy link
Contributor

pelwell commented Jan 5, 2018

I think you forgot to push - the errors I'm expecting you to fix are that boolean properties are referred to as "wakeup-source?" not "wakeup-source:?".

@pelwell
Copy link
Contributor

pelwell commented Jan 5, 2018

While PRs are under development it is often better to keep updates as separate commits so it is clear what has changed at each step. GitHub allows changes to be viewed by commit or by file, which effectively squashes them to see the sum of all changes, and multiple commits can be squashed at merge time.

@shawaj
Copy link
Author

shawaj commented Jan 5, 2018

ok, have removed the colons now

@pelwell pelwell merged commit 2dbf1a9 into raspberrypi:rpi-4.9.y Jan 5, 2018
@pelwell
Copy link
Contributor

pelwell commented Jan 5, 2018

Merged - thanks.

@shawaj
Copy link
Author

shawaj commented Jan 5, 2018

great, thanks :-)

popcornmix added a commit to raspberrypi/firmware that referenced this pull request Jan 8, 2018
kernel: Add additional overrides to rotary-encoder overlay
See: raspberrypi/linux#2334

kernel: An overlay that allows a Linux key to be bound to a GPIO
See: raspberrypi/linux#2329

firmware: dtoverlay app: Keep overlay symbols private
firmware: dtoverlay app: Report unknown parameters in help

firmware: IL ISP: Remove DPCM10_8 compressed input
firmware: mmal_il: Add missing mappings for 8 bit Bayer encodings
firmware: IMX219 tuning: enable motion detection
firmware: IL camera: increase minimum resolution to 32x32

firmware: audioplus: hdmi: Remove spamming logging message
firmware: tidy: Platform cull
popcornmix added a commit to Hexxeh/rpi-firmware that referenced this pull request Jan 8, 2018
kernel: Add additional overrides to rotary-encoder overlay
See: raspberrypi/linux#2334

kernel: An overlay that allows a Linux key to be bound to a GPIO
See: raspberrypi/linux#2329

firmware: dtoverlay app: Keep overlay symbols private
firmware: dtoverlay app: Report unknown parameters in help

firmware: IL ISP: Remove DPCM10_8 compressed input
firmware: mmal_il: Add missing mappings for 8 bit Bayer encodings
firmware: IMX219 tuning: enable motion detection
firmware: IL camera: increase minimum resolution to 32x32

firmware: audioplus: hdmi: Remove spamming logging message
firmware: tidy: Platform cull
TiejunChina pushed a commit to TiejunChina/linux that referenced this pull request Feb 2, 2018
TiejunChina pushed a commit to TiejunChina/linux that referenced this pull request Feb 14, 2018
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.

2 participants