Add support for CH1115 Controller
Subsystems (3)
v2 cover Add support for CH1115 Controller
This series add support for the CH1115 Controller [1]. It is a monochrome dot matrix OLED controller that supports several interfaces such as SPI and I2C.
This driver only supports the I2C interface, but all common parts could easily be put into a common file to be used with other interfaces. I only have I2C to test with.
This is the first driver I write. The work is based on the driver located at:
- drivers/gpu/drm/sitronix/st7571-i2c.c
- drivers/gpu/drm/sitronix/st7571.c
Tested with Raspberry Pi 3b+.
Changelog:
v2:
- general:
- patch series based in kernel v7.2
- in dt-bindings:
- Fixed linter problems
- Replaced first_page property with offset
- In driver code:
- Fixed dst_pitch calculation
- Renamed file from ch1115.c to ch1115-i2c.c
- Support flip-vertical and flip-horizontal dt properties
- Fixed several things found by sashiko-ai
v1: [2]
[1] https://datasheet4u.com/download/1576606/CH1115.html
[2] https://lore.kernel.org/all/[email protected]/
Nicolás Antinori (3):
dt-bindings: display: add Chipwealth CH1115 OLED Controller drm/ch1115: add support for Chipwealth CH1115 OLED controller
MAINTAINERS: add entry for Chipwealth CH1115 OLED Controller
.../bindings/display/chipwealth,ch1115.yaml | 89 ++ .../devicetree/bindings/vendor-prefixes.yaml | 2 + MAINTAINERS | 6 + drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/chipwealth/Kconfig | 16 + drivers/gpu/drm/chipwealth/Makefile | 3 + drivers/gpu/drm/chipwealth/ch1115-i2c.c | 854 ++++++++++++++++++ 8 files changed, 972 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/chipwealth,ch1115.yaml create mode 100644 drivers/gpu/drm/chipwealth/Kconfig create mode 100644 drivers/gpu/drm/chipwealth/Makefile create mode 100644 drivers/gpu/drm/chipwealth/ch1115-i2c.c
--
2.47.3
PATCH v2 1/3 dt-bindings: display: add Chipwealth CH1115 OLED Controller
Chipwealth CH1115 is a controller for monochrome dot matrix OLED panels.
---
NOTE: patch series based in kernel v7.2
.../bindings/display/chipwealth,ch1115.yaml | 89 +++++++++++++++++++ .../devicetree/bindings/vendor-prefixes.yaml | 2 + 2 files changed, 91 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/chipwealth,ch1115.yaml
Documentation/devicetree/bindings/display/chipwealth,ch1115.yaml new file +89 -0
@@ -0,0 +1,89 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/chipwealth,ch1115.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Chipwealth CH1115 Display Controller + +maintainers: + - Nicolás Antinori <[email protected]> + +description: + Chipwealth CH1115 is a controller for monochrome dot matrix OLED + panels. + + https://datasheet4u.com/download/1576606/CH1115.html + +allOf: + - $ref: panel/panel-common.yaml# + +properties: + compatible: + const: chipwealth,ch1115 + + reg: + maxItems: 1 + + chipwealth,invert: + type: boolean + description: + Display pixels are inverted, i.e. 0 is white and 1 is black. + + chipwealth,offset: + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 0 + maximum: 63 + description: + Display start line offset. Specifies the vertical alignment and row shift along the + Y-coordinate. + + chipwealth,contrast: + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 0 + maximum: 255 + description: + Panel contrast. + + width-mm: true + height-mm: true + flip-horizontal: true + flip-vertical: true + panel-timing: true + +required: + - compatible + - reg + - width-mm + - height-mm + - panel-timing + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + status = "okay"; + + display@3c { + compatible = "chipwealth,ch1115"; + reg = <0x3c>; + width-mm = <12>; + height-mm = <8>; + chipwealth,contrast = <127>; + chipwealth,offset = <8>; + panel-timing { + clock-frequency = <0>; + hactive = <88>; + vactive = <48>; + hfront-porch = <0>; + hback-porch = <0>; + hsync-len = <0>; + vsync-len = <0>; + vfront-porch = <0>; + vback-porch = <0>; + }; + }; + };
Documentation/devicetree/bindings/vendor-prefixes.yaml +2 -0
@@ -325,6 +325,8 @@ patternProperties: description: ChipOne "^chipspark,.*": description: ChipSPARK + "^chipwealth,.*": + description: Chip Wealth Technology, Ltd. "^chongzhou,.*": description: Shenzhen Chongzhou Electronic Technology Co., Ltd "^chrontel,.*":
--
2.47.3
[email protected]
AI reviewRe: [PATCH v2 1/3] dt-bindings: display: add Chipwealth CH1115 OLED Controller
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] The binding incorrectly uses `additionalProperties: false` alongside an `allOf` `$ref` to…
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The binding incorrectly uses additionalProperties: false alongside an allOf $ref to panel-common.yaml, preventing the use of standard panel properties. --
--- Patch [1]: [PATCH v2 1/3] dt-bindings: display: add Chipwealth CH1115 OLED Controller --- commit dea96c423e55cd876a4ae165ef9b13a3b42cc6fc
Author: Nicolás Antinori <[email protected]>
dt-bindings: display: add Chipwealth CH1115 OLED Controller
Chipwealth CH1115 is a controller for monochrome dot matrix OLED panels.
Quoted 5 lines
Documentation/devicetree/bindings/display/chipwealth,ch1115.yaml new file +0
-0
[ ... ]
Quoted 4 lines
Documentation/devicetree/bindings/display/chipwealth,ch1115.yaml new file +4
-0+allOf: + - $ref: panel/panel-common.yaml# + +properties:
[ ... ]
Documentation/devicetree/bindings/display/chipwealth,ch1115.yaml line 52
Quoted 12 lines
Documentation/devicetree/bindings/display/chipwealth,ch1115.yaml new file +12
-0+ chipwealth,contrast: + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 0 + maximum: 255 + description: + Panel contrast. + + width-mm: true + height-mm: true + flip-horizontal: true + flip-vertical: true + panel-timing: true
[Severity: Low]
Are these manual redefinitions of standard panel properties necessary?
Since they are already included via panel-common.yaml above, redefining them locally appears to be a workaround for the additionalProperties: false schema evaluation issue noted below.
Documentation/devicetree/bindings/display/chipwealth,ch1115.yaml line 61
Quoted 9 lines
Documentation/devicetree/bindings/display/chipwealth,ch1115.yaml new file +9
-0+ +required: + - compatible + - reg + - width-mm + - height-mm + - panel-timing + +additionalProperties: false
[Severity: Low]
Should this use unevaluatedProperties: false instead?
Using additionalProperties: false alongside an allOf $ref restricts the schema to evaluate only locally defined properties. This causes it to incorrectly reject standard properties included from panel-common.yaml, such as backlight, power-supply, or port.
Changing this to unevaluatedProperties: false allows standard panel properties to be evaluated correctly, which also removes the need to manually redefine them in this file.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Re: [PATCH v2 1/3] dt-bindings: display: add Chipwealth CH1115 OLED Controller
In reply to Nicolás AntinoriDocumentation/devicetree/bindings/display/chipwealth,ch1115.yaml line 28
Quoted 9 lines · Nicolás Antinori
Documentation/devicetree/bindings/display/chipwealth,ch1115.yaml new file +9
-0+ +properties: + compatible: + const: chipwealth,ch1115 + + reg: + maxItems: 1 + + chipwealth,invert:
Do not come with own style.
git grep invert: -- Documentation/devicetree/bindings/display/ git grep inverted: -- Documentation/devicetree/bindings/display/
so chipwealth,inverted.
Documentation/devicetree/bindings/display/chipwealth,ch1115.yaml line 39
Quoted 11 lines
Documentation/devicetree/bindings/display/chipwealth,ch1115.yaml new file +11
-0+ type: boolean + description: + Display pixels are inverted, i.e. 0 is white and 1 is black. + + chipwealth,offset: + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 0 + maximum: 63 + description: + Display start line offset. Specifies the vertical alignment and row shift along the + Y-coordinate.
Please wrap code according to the preferred limit expressed in Kernel coding style (checkpatch is not a coding style description, but only a tool). However don't wrap blindly (see Kernel coding style).
Missing "default:"
Documentation/devicetree/bindings/display/chipwealth,ch1115.yaml line 46
Quoted 7 lines
Documentation/devicetree/bindings/display/chipwealth,ch1115.yaml new file +7
-0+ + chipwealth,contrast: + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 0 + maximum: 255 + description: + Panel contrast.
I do not get why this should be fixed property. Contrast feels like runtime adjustable feature.
Quoted 13 lines
Documentation/devicetree/bindings/display/chipwealth,ch1115.yaml new file +13
-0+ + width-mm: true + height-mm: true + flip-horizontal: true + flip-vertical: true + panel-timing: true + +required: + - compatible + - reg + - width-mm + - height-mm + - panel-timing
Documentation/devicetree/bindings/display/chipwealth,ch1115.yaml line 68
Quoted 10 lines
Documentation/devicetree/bindings/display/chipwealth,ch1115.yaml new file +9
-0+ +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + status = "okay";
Drop, does not exist in bindings.
Please use existing, recently added code as an example/base.
Quoted 11 lines
Documentation/devicetree/bindings/display/chipwealth,ch1115.yaml new file +11
-0+ + display@3c { + compatible = "chipwealth,ch1115"; + reg = <0x3c>; + width-mm = <12>; + height-mm = <8>; + chipwealth,contrast = <127>; + chipwealth,offset = <8>; + panel-timing { + clock-frequency = <0>; + hactive = <88>;
Documentation/devicetree/bindings/display/chipwealth,ch1115.yaml line 89
Quoted 10 lines
Documentation/devicetree/bindings/display/chipwealth,ch1115.yaml new file +10
-0+ vactive = <48>; + hfront-porch = <0>; + hback-porch = <0>; + hsync-len = <0>; + vsync-len = <0>; + vfront-porch = <0>; + vback-porch = <0>; + }; + }; + };
Best regards,
Krzysztof
PATCH v2 2/3 drm/ch1115: add support for Chipwealth CH1115 OLED controller
In reply to Nicolás AntinoriChipwealth CH1115 is a controller for monochrome dot matrix OLED panels.
Add DRM support for this chip using i2c bus.
---
NOTE: patch series based in kernel v7.2
drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/chipwealth/Kconfig | 16 + drivers/gpu/drm/chipwealth/Makefile | 3 + drivers/gpu/drm/chipwealth/ch1115-i2c.c | 854 ++++++++++++++++++++++++ 5 files changed, 875 insertions(+) create mode 100644 drivers/gpu/drm/chipwealth/Kconfig create mode 100644 drivers/gpu/drm/chipwealth/Makefile create mode 100644 drivers/gpu/drm/chipwealth/ch1115-i2c.c
drivers/gpu/drm/Kconfig +1 -0
@@ -307,6 +307,7 @@ source "drivers/gpu/drm/aspeed/Kconfig" source "drivers/gpu/drm/ast/Kconfig" source "drivers/gpu/drm/atmel-hlcdc/Kconfig" source "drivers/gpu/drm/bridge/Kconfig" +source "drivers/gpu/drm/chipwealth/Kconfig" source "drivers/gpu/drm/etnaviv/Kconfig" source "drivers/gpu/drm/exynos/Kconfig" source "drivers/gpu/drm/fsl-dcu/Kconfig"
drivers/gpu/drm/Makefile +1 -0
@@ -238,6 +238,7 @@ obj-$(CONFIG_DRM_SPRD) += sprd/ obj-$(CONFIG_DRM_LOONGSON) += loongson/ obj-$(CONFIG_DRM_POWERVR) += imagination/ obj-$(CONFIG_DRM_VERISILICON_DC) += verisilicon/ +obj-y += chipwealth/ # Ensure drm headers are self-contained and pass kernel-doc hdrtest-files := \
drivers/gpu/drm/chipwealth/Kconfig new file +16 -0
@@ -0,0 +1,16 @@ +# SPDX-License-Identifier: GPL-2.0-only +config DRM_CH1115_I2C + tristate "DRM support for Ch1115 OLED panels (I2C BUS)" + depends on DRM && I2C + select DRM_CLIENT_SELECTION + select DRM_GEM_SHMEM_HELPER + select DRM_KMS_HELPER + select REGMAP_I2C + select VIDEOMODE_HELPERS + help + Chipwealth CH1115 is a controller for monochrome dot matrix OLED + panels. + + DRM Driver for Ch1115 OLED panels connected via I2C. + + If M is selected the module will be called ch1115-i2c.
drivers/gpu/drm/chipwealth/Makefile new file +3 -0
@@ -0,0 +1,3 @@ +# SPDX-License-Identifier: GPL-2.0-only + +obj-$(CONFIG_DRM_CH1115_I2C) += ch1115-i2c.o
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +854 -0
854 lines not on this page · raw message
--
2.47.3
[email protected]
AI reviewRe: [PATCH v2 2/3] drm/ch1115: add support for Chipwealth CH1115 OLED controller
Thank you for your contribution! Sashiko AI review found 10 potential issue(s) to consider: - [High] The driver performs I2C transfers, which are sleeping functions, inside DRM atomic context…
Thank you for your contribution! Sashiko AI review found 10 potential issue(s) to consider:
- [High] The driver performs I2C transfers, which are sleeping functions, inside DRM atomic context callbacks.
- [High] The hwbuf allocation size calculation fails to account for row pitch alignment, leading to heap buffer overflows.
- [High] Missing format conversion state reservation in atomic_check causes a GFP_KERNEL allocation in the atomic update path.
- [High] Missing zero validation for display dimensions can lead to ZERO_SIZE_PTR dereference.
- [High] Missing drm_dev_unplug() causes drm_dev_enter() to fail to protect against Use-After-Free of devm-managed resources.
- [High] Missing drm_dev_enter()/drm_dev_exit() protection in encoder callbacks.
- [High] Improper handling of plane source coordinates causes Out-Of-Bounds memory accesses.
- [Medium] Hardware shutdown commands are sent before unregistering the DRM device, exposing the powered-off device to userspace commits.
- [Medium] Damage clip X coordinates are not rounded to byte boundaries, causing the monochrome conversion helper to corrupt the destination buffer.
- [Medium] CH1115_CMD_OFFSET is programmed with a value of 64, which exceeds the valid hardware limit.
--
--- Patch [2]: [PATCH v2 2/3] drm/ch1115: add support for Chipwealth CH1115 OLED controller --- commit b8a12cbafccb463172c071e5d13f894de430373b
Author: Nicolás Antinori <[email protected]>
drm/ch1115: add support for Chipwealth CH1115 OLED controller
Chipwealth CH1115 is a controller for monochrome dot matrix OLED panels. Add DRM support for this chip using i2c bus.
Quoted 4 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +0
-0
[ ... ]
Quoted 2 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +2
-0+static int ch1115_oled_init (struct ch1115_device *ch1115) +{
[ ... ]
drivers/gpu/drm/chipwealth/ch1115-i2c.c line 196
Quoted 4 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +4
-0+CH1115_CMD_OFFSET , + ch1115->flip_vertical + ? ch1115->y_start_offset + : 64 - ch1115->y_start_offset,
[Severity: Medium]
Is this offset calculation correct for when flip_vertical is false? If y_start_offset is 0, this evaluates to 64. Display offset values for this controller type are 6-bit and must be strictly between 0 and 63. Sending 64 might be out of bounds and could corrupt the internal display state machine.
[ ... ]
Quoted 4 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +4
-0+static void ch1115_prepare_buffer(struct ch1115_device *ch1115, + const struct iosys_map *vmap, + struct drm_framebuffer *fb, + struct drm_rect *rect,
drivers/gpu/drm/chipwealth/ch1115-i2c.c line 243
Quoted 10 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +10
-0+ struct drm_format_conv_state *fmtcnv_state) +{ +unsigned int dst_pitch; +struct iosys_map dst; + + dst_pitch =DIV_ROUND_UP (ch1115->width,8 ); + rect->y1 =round_down (rect->y1,CH1115_PAGE_HEIGHT ); + rect->y2 = min_t(unsigned int, round_up(rect->y2, CH1115_PAGE_HEIGHT), ch1115->height); + +iosys_map_set_vaddr (&dst, ch1115->hwbuf + rect->y1 * dst_pitch + rect->x1 /8 );
[Severity: High]
Could this calculation result in an out-of-bounds heap memory access? The damage rectangles returned by drm_atomic_helper_damage_iter_next are in plane source coordinates. If a user sets a non-zero plane source offset or a framebuffer larger than the panel, rect->x1 and rect->y1 can exceed the panel dimensions, causing the absolute offsets to point outside the hwbuf allocation. These coordinates should be translated to the plane's destination coordinates first.
[Severity: Medium]
Are we guaranteed that rect->x1 and rect->x2 are aligned to byte boundaries? The monochrome conversion helper drm_fb_xrgb8888_to_mono assumes the start pixel maps to bit 0. If rect->x1 is not a multiple of 8, converted pixels will be shifted improperly in the destination byte, corrupting the image and overwriting adjacent pre-existing pixels outside the clip boundary.
Quoted 2 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +2
-0+drm_fb_xrgb8888_to_mono (&dst, &dst_pitch, vmap, fb, rect, fmtcnv_state); +}
[ ... ]
drivers/gpu/drm/chipwealth/ch1115-i2c.c line 286
Quoted 12 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +12
-0+static int ch1115_fb_update_rect (struct drm_framebuffer *fb,struct drm_rect *rect) +{ +struct ch1115_device *ch1115 =drm_to_ch1115 (fb->dev); +char *row = ch1115->row; +int ret; + + rect->y1 =round_down (rect->y1,CH1115_PAGE_HEIGHT ); + rect->y2 = min_t(unsigned int, round_up(rect->y2, CH1115_PAGE_HEIGHT), ch1115->height); + +for (int y = rect->y1; y < rect->y2; y +=CH1115_PAGE_HEIGHT ) { +for (int x = rect->x1; x < rect->x2; x++) + row[x] =ch1115_transform_xy (ch1115->hwbuf, x, y, ch1115->bytes_per_row);
[Severity: High]
Can this index exceed the bounds of the row array? Similar to the hwbuf offset above, rect->x1 and rect->x2 can exceed the panel width if the plane source coordinates are larger than the display, writing out of bounds into the heap.
[ ... ]
Quoted 3 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +3
-0+static int ch1115_primary_plane_helper_atomic_check (struct drm_plane *plane, +struct drm_atomic_commit *state) +{
[ ... ]
drivers/gpu/drm/chipwealth/ch1115-i2c.c line 379
Quoted 5 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +5
-0+return drm_atomic_helper_check_plane_state (new_plane_state, new_crtc_state, +DRM_PLANE_NO_SCALING , +DRM_PLANE_NO_SCALING , + false, false); +}
[Severity: High]
Should we call drm_format_conv_state_reserve here? Because the driver uses drm_fb_xrgb8888_to_mono during the atomic update phase, failing to reserve the format conversion state here (where sleeping is allowed) forces a GFP_KERNEL allocation inside the non-failable atomic update path.
[ ... ]
Quoted 3 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +3
-0+static void ch1115_primary_plane_helper_atomic_update (struct drm_plane *plane, +struct drm_atomic_commit *state) +{
[ ... ]
drivers/gpu/drm/chipwealth/ch1115-i2c.c line 408
Quoted 6 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +6
-0+drm_atomic_helper_damage_iter_init (&iter, old_plane_state, plane_state); +drm_atomic_for_each_plane_damage (&iter, &damage) { +ch1115_prepare_buffer (ch1115, + &shadow_plane_state->data[0 ], + fb, &damage, + &shadow_plane_state->fmtcnv_state);
[Severity: High]
Does this sequence allocate memory in atomic context? Since drm_format_conv_state_reserve is not called in atomic_check, the drm_fb_xrgb8888_to_mono helper (invoked by ch1115_prepare_buffer) will attempt a GFP_KERNEL allocation. This causes a scheduling while atomic panic.
drivers/gpu/drm/chipwealth/ch1115-i2c.c line 410
Quoted 2 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +2
-0+ +ch1115_fb_update_rect (fb, &damage);
[Severity: High]
Can this trigger a sleep while in atomic context? The ch1115_fb_update_rect function issues I2C commands. The DRM subsystem strictly forbids sleeping in plane atomic update callbacks, but i2c_transfer acquires a mutex and will trigger a scheduling while atomic kernel panic.
Quoted 3 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +3
-0+ } + +drm_dev_exit (idx);
[ ... ]
drivers/gpu/drm/chipwealth/ch1115-i2c.c line 525
Quoted 11 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +11
-0+static void ch1115_encoder_atomic_enable (struct drm_encoder *encoder, +struct drm_atomic_commit *state) +{ +struct drm_device *drm = encoder->dev; +struct ch1115_device *ch1115 =drm_to_ch1115 (drm); + +ch1115_oled_init (ch1115); + +u8 command =CH1115_CMD_ON ; + +ch1115_send_command_list (ch1115, &command,1 );
[Severity: High]
Is it safe to issue I2C commands directly in the encoder enable callback? This path executes in atomic context where sleeping is prohibited, but ch1115_send_command_list triggers an i2c_transfer which can sleep.
[Severity: High]
Does this hardware access need drm_dev_enter and drm_dev_exit protection? Unlike the plane update functions, the encoder enable callback accesses the regmap directly without verifying if the device is still plugged in. If an atomic commit races with device unplug, this attempts a use-after-free on the devm-managed regmap.
drivers/gpu/drm/chipwealth/ch1115-i2c.c line 536
Quoted 11 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +11
-0+} + +static void ch1115_encoder_atomic_disable (struct drm_encoder *encoder, +struct drm_atomic_commit *state) +{ +struct drm_device *drm = encoder->dev; +struct ch1115_device *ch1115 =drm_to_ch1115 (drm); + +u8 command =CH1115_CMD_OFF ; + +ch1115_send_command_list (ch1115, &command,1 );
[Severity: High]
Does this sleeping function also violate the atomic context constraints? Just like the enable callback, atomic_disable executes in atomic context and cannot use i2c_transfer.
[Severity: High]
Is the regmap access protected here? Similar to the enable callback, this requires drm_dev_enter and drm_dev_exit to avoid a use-after-free on device unplug.
Quoted 1 line
+}
[ ... ]
Quoted 2 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +2
-0+static int ch1115_parse_dt (struct ch1115_device *ch1115) +{
[ ... ]
drivers/gpu/drm/chipwealth/ch1115-i2c.c line 713
Quoted 8 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +8
-0+if (dt.hactive.typ >CH1115_MAX_WIDTH ) { +dev_err (dev,"width=%d must be less than %d\n" , dt.hactive.typ,CH1115_MAX_WIDTH ); +return -EINVAL ; + } +if (dt.vactive.typ >CH1115_MAX_HEIGHT ) { +dev_err (dev,"height=%d must be less than %d\n" , dt.vactive.typ,CH1115_MAX_HEIGHT ); +return -EINVAL ; + }
[Severity: High]
What happens if the device tree provides 0 for either of these dimensions? The parsing validates the upper bounds but missing a check against zero would cause the hwbuf and row allocations to request 0 bytes. This returns the ZERO_SIZE_PTR (0x10), leading to a kernel panic upon dereference.
[ ... ]
Quoted 2 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +2
-0+static int ch1115_probe (struct i2c_client *client) +{
[ ... ]
drivers/gpu/drm/chipwealth/ch1115-i2c.c line 760
Quoted 3 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +3
-0+ ch1115->hwbuf =devm_kzalloc (&client->dev, + (ch1115->width * ch1115->height) /8 , +GFP_KERNEL );
[Severity: High]
Can this allocation be smaller than the required buffer size? The size is calculated without padding, but ch1115_prepare_buffer converts formats using a pitch of DIV_ROUND_UP(width, 8). If width is not a multiple of 8, drm_fb_xrgb8888_to_mono will write past the end of the allocation.
[ ... ]
Quoted 2 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +2
-0+static void ch1115_remove (struct i2c_client *client) +{
[ ... ]
drivers/gpu/drm/chipwealth/ch1115-i2c.c line 826
Quoted 7 lines
drivers/gpu/drm/chipwealth/ch1115-i2c.c new file +7
-0+ ret =ch1115_send_command_list (ch1115, cmd_list,ARRAY_SIZE (cmd_list)); +if (ret <0 ) +dev_err (&client->dev,"There was an error executing the shutdown commands" ); + +drm_dev_unregister (&ch1115->dev); +drm_atomic_helper_shutdown (&ch1115->dev); +}
[Severity: Medium]
Should drm_dev_unregister be called before turning off the hardware? By shutting down the panel while it is still registered, there is a window where userspace can perform atomic commits on a powered-off display, which could leave the hardware in an inconsistent state.
[Severity: High]
Will using drm_dev_unregister here instead of drm_dev_unplug cause problems for devm allocations? Calling drm_dev_unregister leaves the dev->unplugged flag unset. This allows drm_dev_enter to succeed even after the device is unbound. Delayed atomic commits could then bypass drm_dev_enter and access the freed devm-managed memory.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
PATCH v2 3/3 MAINTAINERS: add entry for Chipwealth CH1115 OLED Controller
In reply to Nicolás AntinoriAdd MAINTAINERS entry for the Chipwealth CH1115 dot matrix OLED controller.
--- MAINTAINERS | 6 ++++++ 1 file changed, 6 insertions(+)
MAINTAINERS +6 -0
@@ -8581,6 +8581,12 @@ T: git https://gitlab.freedesktop.org/drm/misc/kernel.git F: Documentation/devicetree/bindings/display/panel/samsung,lms380kf01.yaml F: drivers/gpu/drm/panel/panel-widechips-ws2401.c +DRM DRIVER FOR CHIPWEALTH CH1115 PANELS +M: Nicolás Antinori <[email protected]> +S: Maintained +F: Documentation/devicetree/bindings/display/chipwealth,ch1115.yaml +F: drivers/gpu/drm/chipwealth/ch1115-i2c.c + DRM DRIVERS M: David Airlie <[email protected]> M: Simona Vetter <[email protected]>
--
2.47.3