Skip to content

Conversation

@DineshDK03
Copy link
Contributor

@DineshDK03 DineshDK03 commented Sep 3, 2021

Added new display driver ek79652

Signed-off-by: Dinesh Kumar K [email protected]

Copy link
Contributor

@mbolivar-nordic mbolivar-nordic left a comment

Choose a reason for hiding this comment

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

Hi and thanks for the patch. A few comments about the devicetree from my side.

Copy link
Contributor

Choose a reason for hiding this comment

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

What's going on here? This is undefined, so the #ifdef GDEW027W3 below is dead code.

If you need to match on a particular piece of hardware, then I think we need to get around to removing CONFIG_LVGL_DISPLAY_DEV_NAME and using a devicetree node instead.

Then you can either:

  1. match on its compatible with DT_NODE_HAS_COMPAT: https://docs.zephyrproject.org/latest/reference/devicetree/api.html#c.DT_NODE_HAS_COMPAT
  2. add some other devicetree property that the driver or application can use to decide to use display_refresh instead of display_blanking_off

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mbolivar-nordic As suggested , Inorder match for this particular hardware, used DT_NODE_HAS_COMPAT.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you need to match on a particular piece of hardware, then I think we need to get around to removing CONFIG_LVGL_DISPLAY_DEV_NAME

This part was important, can you please do that too?

What I mean is that Kconfig options which just specify a device name have been deprecate for a long time now that the devicetree.h API has been stabilized and the device model has been updated to use devicetree nodes.

The thing to do now is to just get the struct device* from a devicetree pointer, using things like DEVICE_DT_GET, DEVICE_DT_GET_ANY, or DEVICE_DT_GET_ONE.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apologies for the delayed reply, if we remove CONFIG_LVGL_DISPLAY_DEV_NAME and get the device using any of the DT api , it will then go into hardware specific, also this type of west build may create conflicts right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the reply, but I do not understand. What conflicts are you referring to?

Copy link
Contributor Author

@DineshDK03 DineshDK03 Nov 22, 2021

Choose a reason for hiding this comment

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

I thought, if --DSHIELD overlay and device name used inside main.c will create conflicts but it wont. So modified the requested changes

@DineshDK03 DineshDK03 force-pushed the ek79652 branch 2 times, most recently from ab79dd8 to 7bf9271 Compare November 12, 2021 10:06
parthitce
parthitce previously approved these changes Nov 16, 2021
erwango
erwango previously approved these changes Nov 18, 2021
Copy link
Member

@erwango erwango left a comment

Choose a reason for hiding this comment

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

+1 on shield part

@DineshDK03 DineshDK03 force-pushed the ek79652 branch 2 times, most recently from 23ff9ad to 58d896d Compare November 23, 2021 18:46
@DineshDK03 DineshDK03 requested a review from dleach02 as a code owner November 23, 2021 18:46
@parthitce parthitce reopened this Jun 13, 2022
@zephyrbot zephyrbot requested a review from gmarull June 13, 2022 08:54
@DineshDK03 DineshDK03 force-pushed the ek79652 branch 2 times, most recently from b1e10fa to 71d55e3 Compare June 13, 2022 10:30
@DineshDK03
Copy link
Contributor Author

DineshDK03 commented Jun 13, 2022

@jfischer-no fixed all the comments you mentioned, except this #38306 (comment) , the below image will explain that issue.

1st image: CS pin keeps LOW while sending data value (display not/won't work)
2nd image: CS pin toggles for each data value (display works)

SPI_buffer_combined
SPI_buffer_split

@github-actions
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@jfischer-no
Copy link
Contributor

@DineshDK03 Please take a look at the latest changes @andysan has made in https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/display/uc81xx.c and https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/display/ssd16xx.c, and try to align your driver with these. In the commit history, I see shield support before the driver, this should be the other way around.

@jfischer-no jfischer-no requested review from andysan and removed request for MaureenHelm, avisconti, carlescufi and galak August 16, 2022 10:06
@andysan
Copy link
Contributor

andysan commented Aug 16, 2022

@DineshDK03 Please take a look at the latest changes @andysan has made in https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/display/uc81xx.c and https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/display/ssd16xx.c, and try to align your driver with these. In the commit history, I see shield support before the driver, this should be the other way around.

I would suggest going even further. As far as I can tell, the register layout of this chip is very close (almost identical) to the UltraChip EPD drivers. A quick look at the data sheet suggests that you can most likely just add a quirk structure for this device in UC81xx and it should mostly just work out of the box. You might want to have a look at #48383 if you go down that route.

@github-actions github-actions bot removed the Stale label Aug 17, 2022
@github-actions
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

Add support for gdew027w3 2.7inch epaper display.

Signed-off-by: Dinesh Kumar K <[email protected]>
Added new display driver ek79652 for 2.7inch epaper display.

Signed-off-by: Dinesh Kumar K <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.