Add Anbernic RG Vita-Pro
Subsystems (6)
v5 cover Add Anbernic RG Vita-Pro
From: Chris Morgan <[email protected]>
Add support for the Anbernic RG Vita-Pro, based on the Rockchip RK3576 SoC.
Changes Since V1:
- Updated documentation filename to matche compatible string.
- Corrected order of allocating IRQ in charger driver and switched to using devm managed function to allocate work queue.
Changes Since V2:
- Corrected issue with naming of panel binding document and added fallback compatible string.
- Switched to devm helpers in panel driver. Note that devm_drm_panel_add() is currently only in the drm-next branch.
- Optimized setting GPIOs for panel driver.
Changes Since V3:
- Fixed numerous issues identifed by Sashiko bot, mainly relating to unclamped or improperly clamped values from within the charger driver.
- Corrected hard-coded values in the panel driver unprepare routine as well as correcting an improperly checked return condition.
Changes Since V4:
- Fixed errors identified by Sashiko bot on charger driver that could result in an improper value being written to the registers.
- Corrected some error handling in the panel driver.
Chris Morgan (6):
dt-bindings: power: supply: sgm41542: document sgm41542
power: supply: sgm41542: Add SG Micro sgm41542 charger
dt-bindings: display: panel: Add Anbernic TD4310 panel drm/panel: anbernic-td4310: Add RG Vita Pro panel
dt-bindings: arm: rockchip: Add Anbernic RG Vita-Pro
arm64: dts: rockchip: Add Anbernic RG Vita-Pro
.../devicetree/bindings/arm/rockchip.yaml | 6 + .../display/panel/anbernic,td4310.yaml | 66 + .../power/supply/sgmicro,sgm41542.yaml | 104 ++ arch/arm64/boot/dts/rockchip/Makefile | 1 + .../rockchip/rk3576-anbernic-rg-vita-pro.dts | 1327 +++++++++++++++++ drivers/gpu/drm/panel/Kconfig | 10 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-anbernic-td4310.c | 256 ++++ drivers/power/supply/Kconfig | 8 + drivers/power/supply/Makefile | 1 + drivers/power/supply/sgm41542_charger.c | 1075 +++++++++++++ 11 files changed, 2855 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/anbernic,td4310.yaml create mode 100644 Documentation/devicetree/bindings/power/supply/sgmicro,sgm41542.yaml create mode 100644 arch/arm64/boot/dts/rockchip/rk3576-anbernic-rg-vita-pro.dts create mode 100644 drivers/gpu/drm/panel/panel-anbernic-td4310.c create mode 100644 drivers/power/supply/sgm41542_charger.c
Signature · 1 line
2.43.0
PATCH v5 1/6 dt-bindings: power: supply: sgm41542: document sgm41542
From: Chris Morgan <[email protected]>
Document the SG Micro sgm41542 battery charger/boost converter. The parameters of input-current-limit-microamp and input-voltage-limit-microvolt are defined as such since they are in common use among multiple bindings currently.
--- .../power/supply/sgmicro,sgm41542.yaml | 104 ++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/supply/sgmicro,sgm41542.yaml
Documentation/devicetree/bindings/power/supply/sgmicro,sgm41542.yaml new file +104 -0
@@ -0,0 +1,104 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/power/supply/sgmicro,sgm41542.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: SGM41542 Battery Charger + +description: + The SGMicro SGM41542 is a single cell battery charger/boost controller. + +maintainers: + - Chris Morgan <[email protected]> + - Xu Shengfei <[email protected]> + +allOf: + - $ref: power-supply.yaml# + +properties: + compatible: + const: sgmicro,sgm41542 + + input-current-limit-microamp: + description: + Optional value to clamp the maximum input current limit to for + the device. If omitted, the default value for the hardware will + be used (2400000). + minimum: 100000 + maximum: 3800000 + + input-voltage-limit-microvolt: + description: + Optional value to clamp the maximum input voltage limit to for + the device. If omitted, the default value for the hardware will + be used (4500000). + minimum: 3900000 + maximum: 12000000 + + interrupts: + maxItems: 1 + + monitored-battery: + description: | + A phandle to a monitored battery node. Values for the following + are used by the driver and if not present will result in default + values being applied: + constant-charge-current-max-microamp + constant-charge-voltage-max-microvolt + charge-term-current-microamp + precharge-current-microamp + + reg: + maxItems: 1 + + regulators: + type: object + properties: + otg-vbus: + type: object + description: OTG boost regulator + $ref: /schemas/regulator/regulator.yaml + unevaluatedProperties: false + additionalProperties: false + +required: + - compatible + - reg + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + #include <dt-bindings/interrupt-controller/irq.h> + + battery: battery { + compatible = "simple-battery"; + constant-charge-current-max-microamp = <10000000>; + constant-charge-voltage-max-microvolt = <4350000>; + precharge-current-microamp = <180000>; + charge-term-current-microamp = <300000>; + }; + + i2c { + #address-cells = <1>; + #size-cells = <0>; + + charger@3b { + compatible = "sgmicro,sgm41542"; + reg = <0x3b>; + input-current-limit-microamp = <3000000>; + input-voltage-limit-microvolt = <4500000>; + interrupt-parent = <&gpio0>; + interrupts = <26 IRQ_TYPE_EDGE_FALLING>; + monitored-battery = <&battery>; + + regulators { + otg-vbus { + regulator-max-microvolt = <5000000>; + regulator-min-microvolt = <5000000>; + }; + }; + }; + };
Signature · 1 line
2.43.0
Re: [PATCH V5 1/6] dt-bindings: power: supply: sgm41542: document sgm41542
Quoted 38 lines · Chris Morgan
From: Chris Morgan <[email protected]>
Document the SG Micro sgm41542 battery charger/boost converter. The parameters of input-current-limit-microamp and input-voltage-limit-microvolt are defined as such since they are in common use among multiple bindings currently.
Signed-off-by Chris Morgan <[email protected]>--- .../power/supply/sgmicro,sgm41542.yaml | 104 ++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/supply/sgmicro,sgm41542.yamlDocumentation/devicetree/bindings/power/supply/sgmicro,sgm41542.yaml new file +19
-0@@ -0,0 +1,104 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/power/supply/sgmicro,sgm41542.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: SGM41542 Battery Charger + +description: + The SGMicro SGM41542 is a single cell battery charger/boost controller. + +maintainers: + - Chris Morgan <[email protected]> + - Xu Shengfei <[email protected]> + +allOf: + - $ref: power-supply.yaml# + +properties:
Documentation/devicetree/bindings/power/supply/sgmicro,sgm41542.yaml line 29
Quoted 10 lines
Documentation/devicetree/bindings/power/supply/sgmicro,sgm41542.yaml new file +10
-0+ compatible: + const: sgmicro,sgm41542 + + input-current-limit-microamp: + description: + Optional value to clamp the maximum input current limit to for + the device. If omitted, the default value for the hardware will + be used (2400000). + minimum: 100000 + maximum: 3800000
Instead of the text,
default: 2400000
Documentation/devicetree/bindings/power/supply/sgmicro,sgm41542.yaml line 37
Quoted 8 lines
Documentation/devicetree/bindings/power/supply/sgmicro,sgm41542.yaml new file +8
-0+ + input-voltage-limit-microvolt: + description: + Optional value to clamp the maximum input voltage limit to for + the device. If omitted, the default value for the hardware will + be used (4500000). + minimum: 3900000 + maximum: 12000000
Same here.
pw-bot: changes-requested
Documentation/devicetree/bindings/power/supply/sgmicro,sgm41542.yaml line 46
Quoted 9 lines
Documentation/devicetree/bindings/power/supply/sgmicro,sgm41542.yaml new file +9
-0+ + interrupts: + maxItems: 1 + + monitored-battery: + description: | + A phandle to a monitored battery node. Values for the following + are used by the driver and if not present will result in default + values being applied:
I saw the wording "The charger uses the following battery properties" elsewhere, and I think that's superior to what's here.
I had a quick check and didn't notice other devices that talked about defaults here. I feel like it's probably a good idea to state in the binding what the defaults are, so that if u-boot or whatever grows a support for this hardware that it uses the same ones.
Cheers,
Conor.
Quoted 61 lines
Documentation/devicetree/bindings/power/supply/sgmicro,sgm41542.yaml new file +58
-0+ constant-charge-current-max-microamp + constant-charge-voltage-max-microvolt + charge-term-current-microamp + precharge-current-microamp + + reg: + maxItems: 1 + + regulators: + type: object + properties: + otg-vbus: + type: object + description: OTG boost regulator + $ref: /schemas/regulator/regulator.yaml + unevaluatedProperties: false + additionalProperties: false + +required: + - compatible + - reg + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + #include <dt-bindings/interrupt-controller/irq.h> + + battery: battery { + compatible = "simple-battery"; + constant-charge-current-max-microamp = <10000000>; + constant-charge-voltage-max-microvolt = <4350000>; + precharge-current-microamp = <180000>; + charge-term-current-microamp = <300000>; + }; + + i2c { + #address-cells = <1>; + #size-cells = <0>; + + charger@3b { + compatible = "sgmicro,sgm41542"; + reg = <0x3b>; + input-current-limit-microamp = <3000000>; + input-voltage-limit-microvolt = <4500000>; + interrupt-parent = <&gpio0>; + interrupts = <26 IRQ_TYPE_EDGE_FALLING>; + monitored-battery = <&battery>; + + regulators { + otg-vbus { + regulator-max-microvolt = <5000000>; + regulator-min-microvolt = <5000000>; + }; + }; + }; + };Signature · 1 line
2.43.0
PATCH v5 2/6 power: supply: sgm41542: Add SG Micro sgm41542 charger
In reply to Chris MorganFrom: Chris Morgan <[email protected]>
Add support for the SG Micro SGM41542 charger/boost converter. Driver was adapted from Rockchip BSP driver [1] and confirmed with vendor datasheet [2].
[1] https://github.com/rockchip-linux/kernel/blob/develop-6.6/drivers/power/supply/sgm41542_charger.c
[2] https://www.sg-micro.de/rect/assets/1e8de70b-657e-4156-be68-a64fdbe8e418/SGM41541_SGM41542.pdf
--- drivers/power/supply/Kconfig | 8 + drivers/power/supply/Makefile | 1 + drivers/power/supply/sgm41542_charger.c | 1074 +++++++++++++++++++++++ 3 files changed, 1084 insertions(+) create mode 100644 drivers/power/supply/sgm41542_charger.c
drivers/power/supply/Kconfig +8 -0
@@ -856,6 +856,14 @@ config CHARGER_RK817 help Say Y to include support for Rockchip RK817 Battery Charger. +config CHARGER_SGM41542 + tristate "SGM41542 charger driver" + depends on I2C + depends on GPIOLIB || COMPILE_TEST + select REGMAP_I2C + help + Say Y to enable support for the SGM41542 battery charger. + config CHARGER_SMB347 tristate "Summit Microelectronics SMB3XX Battery Charger" depends on I2C
drivers/power/supply/Makefile +1 -0
@@ -107,6 +107,7 @@ obj-$(CONFIG_CHARGER_BQ25890) += bq25890_charger.o obj-$(CONFIG_CHARGER_BQ25980) += bq25980_charger.o obj-$(CONFIG_CHARGER_BQ256XX) += bq256xx_charger.o obj-$(CONFIG_CHARGER_RK817) += rk817_charger.o +obj-$(CONFIG_CHARGER_SGM41542) += sgm41542_charger.o obj-$(CONFIG_CHARGER_SMB347) += smb347-charger.o obj-$(CONFIG_CHARGER_TPS65090) += tps65090-charger.o obj-$(CONFIG_CHARGER_TPS65217) += tps65217_charger.o
drivers/power/supply/sgm41542_charger.c new file +1074 -0
1,074 lines not on this page · raw message
Signature · 1 line
2.43.0
[email protected]
AI reviewRe: [PATCH V5 2/6] power: supply: sgm41542: Add SG Micro sgm41542 charger
Thank you for your contribution! Sashiko AI review found 9 potential issue(s) to consider: - [High] Registration of power supply prior to initializing limits causes invalid sysfs clamping. - [High]…
Thank you for your contribution! Sashiko AI review found 9 potential issue(s) to consider:
- [High] Registration of power supply prior to initializing limits causes invalid sysfs clamping.
- [High] Missing cancellation of delayed watchdog work in the shutdown callback.
- [High] NULL pointer dereference when accessing i2c_client_get_device_id() result.
- [High] Optional battery info properties mistakenly initialized to -EINVAL causing undefined behavior.
- [Medium] Incorrect clamp ordering causes the charger voltage to exceed the configured safety maximum.
- [Medium] Bitmask macro used instead of integer time value for watchdog timer.
- [Medium] Unconditional calls to disable_irq_wake() trigger kernel warnings if IRQ wake was never enabled.
- [Medium] Incorrect return of IRQ_NONE after hardware interrupt has already been serviced.
- [Medium] Concurrent sysfs writes can desynchronize software and hardware watchdog states. --
commit d54feec760f967942102292d8970099c3b72fb53
Author: Chris Morgan <[email protected]>
power: supply: sgm41542: Add SG Micro sgm41542 charger
Add support for the SG Micro SGM41542 charger/boost converter. Driver was adapted from Rockchip BSP driver and confirmed with vendor datasheet.
Quoted 4 lines
drivers/power/supply/sgm41542_charger.c new file +0
-0
[ ... ]
Quoted 7 lines
drivers/power/supply/sgm41542_charger.c new file +7
-0+static int sgm4154x_set_chrg_volt (struct sgm4154x_device *sgm,int chrg_volt) +{ +int reg_val; +int ret; + +/* +* Note that the value of 0x01111 represents a "special value"
drivers/power/supply/sgm41542_charger.c line 269
Quoted 10 lines
drivers/power/supply/sgm41542_charger.c new file +10
-0+* corresponding to 4352000uV instead of the expected 4336000uV, +* per the datasheet. All other values are as expected. So not +* only do we need to clamp between max and min values, but +* also clamp anything below 4352000uv to 4304000uv to prevent +* overcharging. +*/ +if (chrg_volt <4352000 ) + chrg_volt =clamp (chrg_volt,SGM4154X_VREG_V_MIN_UV ,4304000 ); + chrg_volt =clamp (chrg_volt,SGM4154X_VREG_V_MIN_UV , sgm->init_data.max_vreg); + reg_val = (chrg_volt -SGM4154X_VREG_V_MIN_UV ) /SGM4154X_VREG_V_STEP_UV ;
[Severity: Medium]
Is there a risk of bypassing the max_vreg limit here? If a user requests 4.5V and the DT max_vreg limit is 4.336V, the first check evaluates to false, skipping the gap clamp. The second clamp reduces the requested voltage to 4.336V, producing reg_val = 15.
The hardware defines this as a special configuration for 4.352V, bypassing the intended limit. Should max_vreg be enforced before applying hardware-specific gap limits?
[ ... ]
Quoted 13 lines
drivers/power/supply/sgm41542_charger.c new file +13
-0+static int sgm4154x_set_watchdog_timer (struct sgm4154x_device *sgm,int time) +{ +u8 reg_val; +int ret; + +if (time ==0 ) + reg_val =SGM4154X_WDT_TIMER_DISABLE ; +else if (time ==40 ) + reg_val =SGM4154X_WDT_TIMER_40S ; +else if (time ==80 ) + reg_val =SGM4154X_WDT_TIMER_80S ; +else + reg_val =SGM4154X_WDT_TIMER_160S ;
[ ... ]
drivers/power/supply/sgm41542_charger.c line 468
Quoted 10 lines
drivers/power/supply/sgm41542_charger.c new file +10
-0+if (time) { +if (!sgm->watchdog_enable) +queue_delayed_work (sgm->sgm_monitor_wq, + &sgm->sgm_delay_work, +msecs_to_jiffies (1000 *5 )); + sgm->watchdog_enable = true; + }else { + sgm->watchdog_enable = false; +sgm4154x_watchdog_timer_reset (sgm); + }
[Severity: Medium]
Could concurrent sysfs writes to the online property cause a desynchronization here?
Because there is no locking in this function, race conditions might leave the hardware watchdog armed while the software state reads false. This could prevent the delayed work from re-arming the watchdog, resulting in unintended device resets.
[ ... ]
drivers/power/supply/sgm41542_charger.c line 595
Quoted 12 lines
drivers/power/supply/sgm41542_charger.c new file +12
-0+static int sgm4154x_charger_set_property (struct power_supply *psy, +enum power_supply_property prop, +const union power_supply_propval *val) +{ +struct sgm4154x_device *sgm =power_supply_get_drvdata (psy); +int ret = -EINVAL ; + +switch (prop) { +case POWER_SUPPLY_PROP_ONLINE : +if (val->intval) { + ret =sgm4154x_enable_charger (sgm); +sgm4154x_set_watchdog_timer (sgm,SGM4154X_WDT_TIMER_40S );
[Severity: Medium]
Does this pass the correct value to the watchdog configuration?
SGM4154X_WDT_TIMER_40S is a bitmask (BIT(4), which is 16). The sgm4154x_set_watchdog_timer() function expects an integer time value like 40 or 80. Passing 16 causes it to fall through to the else block, silently configuring a 160s timeout instead.
[ ... ]
Quoted 8 lines
drivers/power/supply/sgm41542_charger.c new file +8
-0+static irqreturn_t sgm4154x_irq_handler_thread (int irq,void *private) +{ +struct sgm4154x_device *sgm4154x = private; +struct sgm4154x_state state; +int ret; + + ret =sgm4154x_get_state (sgm4154x, &state); +if (ret) {
drivers/power/supply/sgm41542_charger.c line 722
Quoted 10 lines
drivers/power/supply/sgm41542_charger.c new file +10
-0+dev_err (sgm4154x->dev,"get state error!\n" ); +return IRQ_NONE ; + } + sgm4154x->state = state; +if (state.vbus_gd) { +if (sgm4154x->init_data.ilim >=SGM4154X_DEFAULT_INPUT_CUR ) { + ret =sgm4154x_set_input_curr_lim (sgm4154x, sgm4154x->init_data.ilim); +if (ret) { +dev_err (sgm4154x->dev,"set input current error!\n" ); +return IRQ_NONE ;
[Severity: Medium]
Is it safe to return IRQ_NONE here?
Reading the state via sgm4154x_get_state() effectively clears the interrupt on the hardware. If sgm4154x_set_input_curr_lim() subsequently fails, returning IRQ_NONE will cause the kernel's spurious interrupt detector to erroneously count the interrupt as unhandled, which might lead to the IRQ line being permanently disabled.
[ ... ]
Quoted 5 lines
drivers/power/supply/sgm41542_charger.c new file +5
-0+static int sgm4154x_hw_init (struct sgm4154x_device *sgm) +{ +struct power_supply_battery_info *bat_info; +int ret; +u32 val;
[ ... ]
drivers/power/supply/sgm41542_charger.c line 806
Quoted 6 lines
drivers/power/supply/sgm41542_charger.c new file +6
-0+ ret =power_supply_get_battery_info (sgm->charger, &bat_info); +if (!ret) { +if (bat_info->constant_charge_current_max_ua) + sgm->init_data.max_ichg = bat_info->constant_charge_current_max_ua; +if (bat_info->constant_charge_voltage_max_uv) + sgm->init_data.max_vreg = bat_info->constant_charge_voltage_max_uv;
[Severity: High]
Could this initialize the limits to negative values?
Optional battery parameters default to -EINVAL (-22) when not defined in the device tree. Because -22 is truthy, this block will assign -22 to max_vreg. When clamping chrg_volt later, it can produce a negative result that gets bitwise left-shifted, triggering undefined behavior and corrupting hardware registers. Should this verify that the values are non-negative?
[ ... ]
Quoted 5 lines
drivers/power/supply/sgm41542_charger.c new file +5
-0+static int sgm4154x_probe (struct i2c_client *client) +{ +const struct i2c_device_id *id =i2c_client_get_device_id (client); +struct device *dev = &client->dev; +struct sgm4154x_device *sgm;
drivers/power/supply/sgm41542_charger.c line 979
Quoted 10 lines
drivers/power/supply/sgm41542_charger.c new file +10
-0+int ret; + + sgm =devm_kzalloc (dev,sizeof (*sgm),GFP_KERNEL ); +if (!sgm) +return -ENOMEM ; + + sgm->client = client; + sgm->dev = dev; + +strscpy (sgm->model_name, id->name,I2C_NAME_SIZE );
[Severity: High]
Is there a risk of a NULL pointer dereference here?
If the driver is bound via sysfs or dynamically instantiated without an explicit match in sgm4154x_i2c_ids, i2c_client_get_device_id() returns NULL, leading to a crash on id->name.
[ ... ]
Quoted 5 lines
drivers/power/supply/sgm41542_charger.c new file +5
-0+ sgm->sgm_monitor_wq =devm_alloc_ordered_workqueue (dev,"%s" , +WQ_MEM_RECLAIM |WQ_FREEZABLE ,"sgm-monitor-wq" ); +if (!sgm->sgm_monitor_wq) +return -EINVAL ; +
drivers/power/supply/sgm41542_charger.c line 1008
Quoted 10 lines
drivers/power/supply/sgm41542_charger.c new file +10
-0+ ret =devm_delayed_work_autocancel (dev, &sgm->sgm_delay_work, + sgm_charger_work); +if (ret) +return dev_err_probe (dev, ret,"Unable to register delayed work\n" ); + + ret =sgm4154x_power_supply_init (sgm, dev); +if (ret) +return dev_err_probe (dev, ret,"Failed to register power supply\n" ); + + ret =sgm4154x_hw_init (sgm);
[Severity: High]
Does registering the power supply before initializing the hardware limits expose uninitialized sysfs attributes?
The sgm->init_data is zero-initialized at this point. If userspace writes to a property like CONSTANT_CHARGE_VOLTAGE during this window, it will clamp against a max_vreg of 0. Subtracting the minimum limit from 0 yields a negative number that gets bitwise left-shifted, causing undefined behavior.
[ ... ]
drivers/power/supply/sgm41542_charger.c line 1029
Quoted 6 lines
drivers/power/supply/sgm41542_charger.c new file +6
-0+ ret =sgm4154x_vbus_regulator_register (sgm); +if (ret) { +disable_irq_wake (client->irq); +return dev_err_probe (dev, ret, +"Unable to register VBUS regulator\n" ); + }
[Severity: Medium]
Can this trigger a WARN_ON if the irq was never provided?
The enable_irq_wake() call earlier is guarded by checking if client->irq is non-zero, but this disable_irq_wake() call is unconditional. If the device probes without an IRQ, this will drop the wake depth below zero.
[ ... ]
drivers/power/supply/sgm41542_charger.c line 1040
Quoted 7 lines
drivers/power/supply/sgm41542_charger.c new file +7
-0+static void sgm4154x_charger_shutdown (struct i2c_client *client) +{ +struct sgm4154x_device *sgm =i2c_get_clientdata (client); + +sgm4154x_set_prechrg_curr (sgm,SGM4154X_PRECHRG_I_DEF_UA ); +sgm4154x_disable_charger (sgm); +}
[Severity: High]
Does the shutdown sequence need to cancel the delayed work?
Since devres teardown is not called on shutdown, sgm_delay_work remains active. If it fires after the I2C controller has suspended, it could attempt transactions on an inactive bus, causing timeouts or hangs during system halt.
drivers/power/supply/sgm41542_charger.c line 1045
Quoted 5 lines
drivers/power/supply/sgm41542_charger.c new file +5
-0+ +static void sgm4154x_charger_remove (struct i2c_client *client) +{ +disable_irq_wake (client->irq); +}
[Severity: Medium]
Similar to the probe error path, is it safe to unconditionally call disable_irq_wake() here? If the device was instantiated without an IRQ, this could trigger a WARN_ON in the IRQ core.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
PATCH v5 3/6 dt-bindings: display: panel: Add Anbernic TD4310 panel
In reply to Chris MorganFrom: Chris Morgan <[email protected]>
The panel used by Anbernic in the RG Vita-Pro is a DSI panel based on the TD4310 controller IC. It measures approximately 5.5 inches diagonally and is 1080x1920 in resolution.
--- .../display/panel/anbernic,td4310.yaml | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/anbernic,td4310.yaml
Documentation/devicetree/bindings/display/panel/anbernic,td4310.yaml new file +66 -0
@@ -0,0 +1,66 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/panel/anbernic,td4310.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Anbernic TD4310 Based Panels + +maintainers: + - Chris Morgan <[email protected]> + +description: + Anbernic TD4310 Based Panels, such as the RG-Vita-Pro panel + (a 1080x1920 5.5 inch panel). + +allOf: + - $ref: panel-common.yaml# + +properties: + compatible: + items: + - enum: + - anbernic,panel-vita-pro + - const: anbernic,td4310 + + reg: + maxItems: 1 + + vdd-supply: + description: Panel power supply + +required: + - compatible + - port + - reg + - reset-gpios + - vdd-supply + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + + dsi { + #address-cells = <1>; + #size-cells = <0>; + + panel@0 { + compatible = "anbernic,panel-vita-pro", "anbernic,td4310"; + reg = <0>; + backlight = <&backlight>; + enable-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>; + reset-gpios = <&gpio0 13 GPIO_ACTIVE_LOW>; + rotation = <270>; + vdd-supply = <&vdd_lcd>; + + port { + panel_in: endpoint { + remote-endpoint = <&dsi_out>; + }; + }; + }; + }; + +...
Signature · 1 line
2.43.0
Re: [PATCH V5 3/6] dt-bindings: display: panel: Add Anbernic TD4310 panel
Quoted 67 lines · Chris Morgan
From: Chris Morgan <[email protected]>
The panel used by Anbernic in the RG Vita-Pro is a DSI panel based on the TD4310 controller IC. It measures approximately 5.5 inches diagonally and is 1080x1920 in resolution.
Signed-off-by Chris Morgan <[email protected]>--- .../display/panel/anbernic,td4310.yaml | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/anbernic,td4310.yamlDocumentation/devicetree/bindings/display/panel/anbernic,td4310.yaml new file +49
-0@@ -0,0 +1,66 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/panel/anbernic,td4310.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Anbernic TD4310 Based Panels + +maintainers: + - Chris Morgan <[email protected]> + +description: + Anbernic TD4310 Based Panels, such as the RG-Vita-Pro panel + (a 1080x1920 5.5 inch panel). + +allOf: + - $ref: panel-common.yaml# + +properties: + compatible: + items: + - enum: + - anbernic,panel-vita-pro + - const: anbernic,td4310 + + reg: + maxItems: 1 + + vdd-supply: + description: Panel power supply + +required: + - compatible + - port + - reg + - reset-gpios + - vdd-supply + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + + dsi { + #address-cells = <1>; + #size-cells = <0>; + + panel@0 {
Documentation/devicetree/bindings/display/panel/anbernic,td4310.yaml line 59
Quoted 10 lines
Documentation/devicetree/bindings/display/panel/anbernic,td4310.yaml new file +10
-0+ compatible = "anbernic,panel-vita-pro", "anbernic,td4310"; + reg = <0>; + backlight = <&backlight>; + enable-gpios = <&gpio0 9 GPIO_ACTIVE_HIGH>; + reset-gpios = <&gpio0 13 GPIO_ACTIVE_LOW>; + rotation = <270>; + vdd-supply = <&vdd_lcd>; + + port { + panel_in: endpoint {
Drop the label if you respin.
pw-bot: not-applicable
Quoted 10 lines
Documentation/devicetree/bindings/display/panel/anbernic,td4310.yaml new file +7
-0+ remote-endpoint = <&dsi_out>; + }; + }; + }; + }; + +...Signature · 1 line
2.43.0
PATCH v5 4/6 drm/panel: anbernic-td4310: Add RG Vita Pro panel
In reply to Chris MorganFrom: Chris Morgan <[email protected]>
The panel used by Anbernic in the RG Vita-Pro is a DSI panel based on the TD4310 controller IC. It measures approximately 5.5 inches diagonally and is 1080x1920 in resolution.
--- drivers/gpu/drm/panel/Kconfig | 10 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-anbernic-td4310.c | 256 ++++++++++++++++++ 3 files changed, 267 insertions(+) create mode 100644 drivers/gpu/drm/panel/panel-anbernic-td4310.c
drivers/gpu/drm/panel/Kconfig +10 -0
@@ -17,6 +17,16 @@ config DRM_PANEL_ABT_Y030XX067A Y030XX067A 320x480 3.0" panel as found in the YLM RG-280M, RG-300 and RG-99 handheld gaming consoles. +config DRM_PANEL_ANBERNIC_TD4310 + tristate "Anbernic TD4310 LCD panel" + depends on GPIOLIB && OF + depends on DRM_MIPI_DSI + depends on BACKLIGHT_CLASS_DEVICE + help + Say Y here to enable support for Anbernic designed panels with the + TD4310 panel controller such as the ones used on the Anbernic RG + Vita Pro. + config DRM_PANEL_ARM_VERSATILE tristate "ARM Versatile panel driver" depends on OF
drivers/gpu/drm/panel/Makefile +1 -0
@@ -1,5 +1,6 @@ # SPDX-License-Identifier: GPL-2.0 obj-$(CONFIG_DRM_PANEL_ABT_Y030XX067A) += panel-abt-y030xx067a.o +obj-$(CONFIG_DRM_PANEL_ANBERNIC_TD4310) += panel-anbernic-td4310.o obj-$(CONFIG_DRM_PANEL_ARM_VERSATILE) += panel-arm-versatile.o obj-$(CONFIG_DRM_PANEL_ASUS_Z00T_TM5P5_NT35596) += panel-asus-z00t-tm5p5-n35596.o obj-$(CONFIG_DRM_PANEL_AUO_A030JTN01) += panel-auo-a030jtn01.o
drivers/gpu/drm/panel/panel-anbernic-td4310.c new file +256 -0
256 lines not on this page · raw message
Signature · 1 line
2.43.0
[email protected]
AI reviewRe: [PATCH V5 4/6] drm/panel: anbernic-td4310: Add RG Vita Pro panel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] The driver calls an undefined function `devm_drm_panel_add()`, causing a compilation failure.…
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The driver calls an undefined function devm_drm_panel_add(), causing a compilation failure. Furthermore, the driver lacks a .remove callback (or a devres action) to call drm_panel_remove(). If the build error is fixed by trivially substituting the standard drm_panel_add(), it will result in a Use-After-Free vulnerability. --
commit 15ea49b70a00236df8f23333ccc634c4fbaf642e
Author: Chris Morgan <[email protected]>
drm/panel: anbernic-td4310: Add RG Vita Pro panel
This commit adds support for the TD4310 controller IC DSI panel used in the Anbernic RG Vita-Pro.
Quoted 5 lines
drivers/gpu/drm/panel/panel-anbernic-td4310.c new file +0
-0
[ ... ]
Quoted 2 lines
drivers/gpu/drm/panel/panel-anbernic-td4310.c new file +2
-0+static int panel_anbernic_td4310_probe (struct mipi_dsi_device *dsi) +{
[ ... ]
drivers/gpu/drm/panel/panel-anbernic-td4310.c line 197
Quoted 5 lines
drivers/gpu/drm/panel/panel-anbernic-td4310.c new file +5
-0+ ret =drm_panel_of_backlight (&ctx->panel); +if (ret) +return ret; + +devm_drm_panel_add (dev, &ctx->panel);
[Severity: High]
Does devm_drm_panel_add() exist in the kernel? It looks like this might cause a build failure due to an undefined function.
If this is resolved by simply replacing it with drm_panel_add(), could it result in a use-after-free on unbind? Because the panel is allocated with devm_drm_panel_alloc() earlier, its memory will be automatically freed when the device is unbound. If drm_panel_remove() is never called, the freed panel would remain in the global DRM panel list, risking memory corruption when other components traverse the list.
[ ... ]
drivers/gpu/drm/panel/panel-anbernic-td4310.c line 251
Quoted 7 lines
drivers/gpu/drm/panel/panel-anbernic-td4310.c new file +7
-0+static struct mipi_dsi_driver anbernic_panel_td4310_driver = { + .driver = { + .name ="panel-anbernic-td4310" , + .of_match_table = panel_anbernic_td4310_of_match, + }, + .probe = panel_anbernic_td4310_probe, +};
[Severity: High]
Would it make sense to add a .remove callback here to call drm_panel_remove(), or perhaps register a custom devm action during probe, to ensure the panel is properly unregistered from the subsystem on teardown?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
Re: [PATCH V5 4/6] drm/panel: anbernic-td4310: Add RG Vita Pro panel
In reply to Chris MorganQuoted 298 lines · Chris Morgan
From: Chris Morgan <[email protected]>
The panel used by Anbernic in the RG Vita-Pro is a DSI panel based on the TD4310 controller IC. It measures approximately 5.5 inches diagonally and is 1080x1920 in resolution.
Signed-off-by Chris Morgan <[email protected]>--- drivers/gpu/drm/panel/Kconfig | 10 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-anbernic-td4310.c | 256 ++++++++++++++++++ 3 files changed, 267 insertions(+)create mode 100644
drivers/gpu/drm/panel/panel-anbernic-td4310.cdrivers/gpu/drm/panel/Kconfig +10
-0@@ -17,6 +17,16 @@ config DRM_PANEL_ABT_Y030XX067A Y030XX067A 320x480 3.0" panel as found in the YLM RG-280M, RG-300 and RG-99 handheld gaming consoles. +config DRM_PANEL_ANBERNIC_TD4310 + tristate "Anbernic TD4310 LCD panel" + depends on GPIOLIB && OF + depends on DRM_MIPI_DSI + depends on BACKLIGHT_CLASS_DEVICE + help + Say Y here to enable support for Anbernic designed panels with the + TD4310 panel controller such as the ones used on the Anbernic RG + Vita Pro. + config DRM_PANEL_ARM_VERSATILE tristate "ARM Versatile panel driver" depends on OFdrivers/gpu/drm/panel/Makefile +1
-0@@ -1,5 +1,6 @@ # SPDX-License-Identifier: GPL-2.0 obj-$(CONFIG_DRM_PANEL_ABT_Y030XX067A) += panel-abt-y030xx067a.o +obj-$(CONFIG_DRM_PANEL_ANBERNIC_TD4310) += panel-anbernic-td4310.o obj-$(CONFIG_DRM_PANEL_ARM_VERSATILE) += panel-arm-versatile.o obj-$(CONFIG_DRM_PANEL_ASUS_Z00T_TM5P5_NT35596) += panel-asus-z00t-tm5p5-n35596.o obj-$(CONFIG_DRM_PANEL_AUO_A030JTN01) += panel-auo-a030jtn01.odrivers/gpu/drm/panel/panel-anbernic-td4310.c new file +246
-0@@ -0,0 +1,256 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Driver for Anbernic panels with TD4310 panel controller. +* +* Copyright (C) 2026 Chris Morgan <[email protected]> +* +*/ + +#include <linux/gpio/consumer.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/regulator/consumer.h> + +#include <drm/drm_mipi_dsi.h> +#include <drm/drm_panel.h> +#include <drm/drm_probe_helper.h> + +#include <video/mipi_display.h> + +struct anbernic_panel_td4310_info { +const struct drm_display_mode *display_mode; +u16 width_mm; +u16 height_mm; +u32 bus_flags; +unsigned long mode_flags; +u32 format; +u32 lanes; +u16 prepare_delay; +u16 reset_delay; +u16 init_delay; +u16 enable_delay; +u16 disable_delay; +u16 unprepare_delay; +}; + +struct anbernic_panel_td4310 { +struct device *dev; +struct mipi_dsi_device *dsi; +struct drm_panel panel; +const struct anbernic_panel_td4310_info *panel_info; +struct gpio_desc *reset_gpio; +struct gpio_desc *enable_gpio; +struct regulator *vdd; +enum drm_panel_orientation orientation; +}; + +static inline struct anbernic_panel_td4310 *panel_to_anbernic_panel_td4310 (struct drm_panel *panel) +{ + return container_of(panel, struct anbernic_panel_td4310, panel); +} + +static int panel_anbernic_td4310_prepare (struct drm_panel *panel) +{ +struct anbernic_panel_td4310 *ctx =panel_to_anbernic_panel_td4310 (panel); +struct mipi_dsi_device *dsi = ctx->dsi; +struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi }; +int ret; + + ret =regulator_enable (ctx->vdd); +if (ret) +return ret; + + ret =gpiod_set_value_cansleep (ctx->enable_gpio,1 ); +if (ret) + goto err_enable; + +gpiod_set_value_cansleep (ctx->reset_gpio,0 ); +mipi_dsi_msleep (&dsi_ctx,10 ); + + ret =gpiod_set_value_cansleep (ctx->reset_gpio,1 ); +if (ret) + goto err_reset; +if (ctx->panel_info->reset_delay) +mipi_dsi_msleep (&dsi_ctx, ctx->panel_info->reset_delay); + ret =gpiod_set_value_cansleep (ctx->reset_gpio,0 ); +if (ret) + goto err_reset; + +if (ctx->panel_info->enable_delay) +mipi_dsi_msleep (&dsi_ctx, ctx->panel_info->enable_delay); + +mipi_dsi_dcs_exit_sleep_mode_multi (&dsi_ctx); +mipi_dsi_msleep (&dsi_ctx, ctx->panel_info->prepare_delay); +mipi_dsi_dcs_set_display_on_multi (&dsi_ctx); +mipi_dsi_msleep (&dsi_ctx, ctx->panel_info->prepare_delay); + +if (dsi_ctx.accum_err) { + ret = dsi_ctx.accum_err; + goto err_reset; + } + +return 0 ; + +err_reset: +gpiod_set_value_cansleep (ctx->enable_gpio,0 ); +err_enable: +regulator_disable (ctx->vdd); +return ret; +} + +static int panel_anbernic_td4310_unprepare (struct drm_panel *panel) +{ +struct anbernic_panel_td4310 *ctx =panel_to_anbernic_panel_td4310 (panel); +struct mipi_dsi_device *dsi = ctx->dsi; +struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi }; + +mipi_dsi_dcs_set_display_off_multi (&dsi_ctx); +mipi_dsi_msleep (&dsi_ctx, ctx->panel_info->unprepare_delay); +mipi_dsi_dcs_enter_sleep_mode_multi (&dsi_ctx); +mipi_dsi_msleep (&dsi_ctx, ctx->panel_info->disable_delay); + +gpiod_set_value_cansleep (ctx->enable_gpio,0 ); + +gpiod_set_value_cansleep (ctx->reset_gpio,1 ); + +regulator_disable (ctx->vdd); + +return 0 ; +} + +static int panel_anbernic_td4310_get_mode (struct drm_panel *panel, +struct drm_connector *connector) +{ +struct anbernic_panel_td4310 *ctx =panel_to_anbernic_panel_td4310 (panel); +const struct anbernic_panel_td4310_info *panel_info = ctx->panel_info; + + connector->display_info.bpc =8 ; + connector->display_info.width_mm = panel_info->width_mm; + connector->display_info.height_mm = panel_info->height_mm; + connector->display_info.bus_flags = panel_info->bus_flags; + +return drm_connector_helper_get_modes_fixed (connector, panel_info->display_mode); +} + +static enum drm_panel_orientation panel_anbernic_td4310_get_orientation (struct drm_panel *panel) +{ +struct anbernic_panel_td4310 *ctx =panel_to_anbernic_panel_td4310 (panel); + +return ctx->orientation; +} + +static const struct drm_panel_funcs panel_anbernic_td4310_funcs = { + .prepare = panel_anbernic_td4310_prepare, + .unprepare = panel_anbernic_td4310_unprepare, + .get_modes = panel_anbernic_td4310_get_mode, + .get_orientation = panel_anbernic_td4310_get_orientation, +}; + +static int panel_anbernic_td4310_probe (struct mipi_dsi_device *dsi) +{ +struct device *dev = &dsi->dev; +struct anbernic_panel_td4310 *ctx; +int ret; + + ctx = devm_drm_panel_alloc(dev, struct anbernic_panel_td4310, panel, + &panel_anbernic_td4310_funcs, +DRM_MODE_CONNECTOR_DSI ); +if (IS_ERR (ctx)) +return PTR_ERR (ctx); + + ctx->dev = dev; + + ctx->panel_info =of_device_get_match_data (dev); +if (!ctx->panel_info) +return -EINVAL ; + + ret =of_drm_get_panel_orientation (dev->of_node, &ctx->orientation); +if (ret <0 ) +return dev_err_probe (dev, ret,"Failed to get panel orientation\n" ); + + ctx->reset_gpio =devm_gpiod_get (dev,"reset" ,GPIOD_OUT_LOW ); +if (IS_ERR (ctx->reset_gpio)) +return dev_err_probe (dev,PTR_ERR (ctx->reset_gpio), +"Cannot get reset gpio\n" ); + + ctx->enable_gpio =devm_gpiod_get_optional (dev,"enable" ,GPIOD_OUT_LOW ); +if (IS_ERR (ctx->enable_gpio)) +return dev_err_probe (dev,PTR_ERR (ctx->enable_gpio), +"Cannot get enable gpio\n" ); + + ctx->vdd =devm_regulator_get (dev,"vdd" ); +if (IS_ERR (ctx->vdd)) +return dev_err_probe (dev,PTR_ERR (ctx->vdd), +"Failed to request vdd regulator\n" ); + + ctx->dsi = dsi; +mipi_dsi_set_drvdata (dsi, ctx); + + dsi->lanes = ctx->panel_info->lanes; + dsi->format = ctx->panel_info->format; + dsi->mode_flags = ctx->panel_info->mode_flags; + + ret =drm_panel_of_backlight (&ctx->panel); +if (ret) +return ret; + +devm_drm_panel_add (dev, &ctx->panel); + + ret =devm_mipi_dsi_attach (dev, dsi); +if (ret <0 ) +return dev_err_probe (dev, ret,"Failed to attach to DSI host\n" ); + +return 0 ; +} + +static const struct drm_display_mode anbernic_vitapro_mode = { + .clock =140020 , + .hdisplay =1080 , + .hsync_start =1080 +50 , + .hsync_end =1080 +50 +4 , + .htotal =1080 +50 +4 +50 , + .vdisplay =1920 , + .vsync_start =1920 +15 , + .vsync_end =1920 +15 +4 , + .vtotal =1920 +15 +4 +32 , + .flags =DRM_MODE_FLAG_NHSYNC |DRM_MODE_FLAG_NVSYNC , +}; + +static const struct anbernic_panel_td4310_info anbernic_vitapro_info = { + .display_mode = &anbernic_vitapro_mode, + .width_mm =69 , + .height_mm =121 , + .bus_flags =DRM_BUS_FLAG_DE_LOW |DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE , + .mode_flags =MIPI_DSI_MODE_VIDEO |MIPI_DSI_MODE_VIDEO_BURST | +MIPI_DSI_MODE_LPM |MIPI_DSI_MODE_NO_EOT_PACKET | +MIPI_DSI_CLOCK_NON_CONTINUOUS , + .format =MIPI_DSI_FMT_RGB888 , + .lanes =4 , + .prepare_delay =50 , + .reset_delay =220 , + .enable_delay =120 , + .disable_delay =50 , + .unprepare_delay =20 , +}; + +static const struct of_device_id panel_anbernic_td4310_of_match[] = { + { + .compatible ="anbernic,panel-vita-pro" , + .data = &anbernic_vitapro_info, + }, + { } +}; +MODULE_DEVICE_TABLE (of, panel_anbernic_td4310_of_match); + +static struct mipi_dsi_driver anbernic_panel_td4310_driver = { + .driver = {
drivers/gpu/drm/panel/panel-anbernic-td4310.c line 256
Quoted 10 lines
drivers/gpu/drm/panel/panel-anbernic-td4310.c new file +10
-0+ .name = "panel-anbernic-td4310", + .of_match_table = panel_anbernic_td4310_of_match, + }, + .probe = panel_anbernic_td4310_probe, +}; +module_mipi_dsi_driver (anbernic_panel_td4310_driver); + +MODULE_AUTHOR ("Chris Morgan <[email protected]>" ); +MODULE_DESCRIPTION ("DRM driver for Anbernic TD4310 MIPI DSI panels" ); +MODULE_LICENSE ("GPL" );
Thanks,
Neil
PATCH v5 5/6 dt-bindings: arm: rockchip: Add Anbernic RG Vita-Pro
In reply to Chris MorganFrom: Chris Morgan <[email protected]>
Add compatible string for the Anbernic RG Vita-Pro.
--- Documentation/devicetree/bindings/arm/rockchip.yaml | 6 ++++++ 1 file changed, 6 insertions(+)
Documentation/devicetree/bindings/arm/rockchip.yaml +6 -0
@@ -66,6 +66,12 @@ properties: - anbernic,rg-ds - const: rockchip,rk3568 + - description: Anbernic RK3576 Handheld Gaming Console + items: + - enum: + - anbernic,rg-vita-pro + - const: rockchip,rk3576 + - description: Ariaboard Photonicat items: - const: ariaboard,photonicat
Signature · 1 line
2.43.0
PATCH v5 6/6 arm64: dts: rockchip: Add Anbernic RG Vita-Pro
In reply to Chris MorganFrom: Chris Morgan <[email protected]>
Add device tree for the Anbernic RG Vita-Pro, based on the Rockchip RK3576. All hardware is currently working except for the accelerometer, the touchscreen, the battery monitor, and DisplayPort over USB-C. PWM is also missing currently, and as a result the LEDs and panel backlight are currently controlled via GPIO but will be changed to PWM once supported.
The Anbernic RG Vita-Pro includes the following hardware:
- A 1080x1920 DSI display with touch.
- 21 buttons.
- 2 Analog joysticks controlled via userspace.
- 3 LEDs.
- 64GB eMMC, 2 SDMMC slots.
- RTL8852BE WiFi (with Bluetooth via UART)
- 1 USB 3.0 USB-C port in OTG mode.
- 3.5mm headphone jack with play button support.
- 5000mAH battery
The following hardware has incomplete driver support and is not yet working:
- An Invensense icm42607p accelerometer.
- A Cellwise cw221x battery monitor.
- A Synaptics i2c touchscreen.
- DisplayPort over USB-C alt-mode.
- PWM controller.
--- arch/arm64/boot/dts/rockchip/Makefile | 1 + .../rockchip/rk3576-anbernic-rg-vita-pro.dts | 1327 +++++++++++++++++ 2 files changed, 1328 insertions(+) create mode 100644 arch/arm64/boot/dts/rockchip/rk3576-anbernic-rg-vita-pro.dts
arch/arm64/boot/dts/rockchip/Makefile +1 -0
@@ -165,6 +165,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-wolfvision-pf5.dtb dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-wolfvision-pf5-display-vz.dtbo dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3568-wolfvision-pf5-io-expander.dtbo dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3576-100ask-dshanpi-a1.dtb +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3576-anbernic-rg-vita-pro.dtb dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3576-armsom-sige5.dtb dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3576-armsom-sige5-v1.2-wifibt.dtbo dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3576-evb1-v10.dtb
arch/arm64/boot/dts/rockchip/rk3576-anbernic-rg-vita-pro.dts new file +1327 -0
1,327 lines not on this page · raw message
Signature · 1 line
2.43.0
[email protected]
AI reviewRe: [PATCH V5 6/6] arm64: dts: rockchip: Add Anbernic RG Vita-Pro
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] USB Type-C source PDO advertises 3A, exceeding the SGM41542 charger's OTG boost capability.…
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] USB Type-C source PDO advertises 3A, exceeding the SGM41542 charger's OTG boost capability.
- [Medium] Missing SPI child device node prevents userspace control of the analog joysticks.
- [Medium] Ghost HDMI configuration exists for a device that lacks an HDMI port.
--
commit 9a10b9538aecda51a72bf0a4aa5290b693df3a4b
Author: Chris Morgan <[email protected]>
arm64: dts: rockchip: Add Anbernic RG Vita-Pro
This commit adds the device tree for the Anbernic RG Vita-Pro, based on the Rockchip RK3576. It defines the hardware configuration including the display, buttons, analog joysticks, battery, and USB-C.
Quoted 4 lines
arch/arm64/boot/dts/rockchip/rk3576-anbernic-rg-vita-pro.dts new file +0
-0
[ ... ]
Quoted 10 lines
arch/arm64/boot/dts/rockchip/rk3576-anbernic-rg-vita-pro.dts new file +10
-0+ hdmi-con { + compatible = "hdmi-connector"; + type = "c"; + + port { + hdmi_con_in: endpoint { + remote-endpoint = <&hdmi_out_con>; + }; + }; + };
[ ... ]
arch/arm64/boot/dts/rockchip/rk3576-anbernic-rg-vita-pro.dts
Quoted 3 lines
+&hdmi {
arch/arm64/boot/dts/rockchip/rk3576-anbernic-rg-vita-pro.dts new file +2
-0+ status = "okay"; +};
[Severity: Medium]
The commit message lists "DisplayPort over USB-C alt-mode" for video output but does not mention an HDMI port. Does this device physically have an HDMI port, or is this a leftover ghost configuration?
Leaving the HDMI pipeline enabled might waste resources or cause audio/display routing confusion in userspace if the port doesn't exist on the hardware.
[ ... ]
Quoted 3 lines
arch/arm64/boot/dts/rockchip/rk3576-anbernic-rg-vita-pro.dts new file +3
-0+ sgm41542: charger@3b { + compatible = "sgmicro,sgm41542"; + reg = <0x3b>;
[ ... ]
Quoted 12 lines
arch/arm64/boot/dts/rockchip/rk3576-anbernic-rg-vita-pro.dts new file +12
-0+ regulators { + vbus5v0_typec: otg-vbus { + regulator-name = "vbus5v0_typec"; + regulator-max-microvolt = <5000000>; + regulator-min-microvolt = <5000000>; + + regulator-state-mem { + regulator-off-in-suspend; + }; + }; + }; + };
[ ... ]
Quoted 2 lines
arch/arm64/boot/dts/rockchip/rk3576-anbernic-rg-vita-pro.dts new file +2
-0+ husb311: typec-portc@4e { + compatible = "hynetek,husb311", "richtek,rt1711h";
[ ... ]
arch/arm64/boot/dts/rockchip/rk3576-anbernic-rg-vita-pro.dts line 954
Quoted 13 lines
arch/arm64/boot/dts/rockchip/rk3576-anbernic-rg-vita-pro.dts new file +13
-0+ vbus-supply = <&vbus5v0_typec>; + + connector { + compatible = "usb-c-connector"; + label = "USB-C"; + data-role = "dual"; + op-sink-microwatt = <10000000>; + power-role = "dual"; + try-power-role = "sink"; + + sink-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM) + PDO_FIXED(9000, 2000, PDO_FIXED_USB_COMM)>; + source-pdos = <PDO_FIXED(5000, 3000, PDO_FIXED_USB_COMM)>;
[Severity: High]
Does advertising 3A for the source PDO risk overloading the SGM41542 charger?
The husb311 Type-C port advertises 5V at 3A to connected sinks, but its vbus-supply is powered by the sgm41542 charger's OTG boost converter. According to the sgm41542 driver, its maximum OTG output capability is physically limited to 2A (2,000,000 uA).
If a sink attempts to draw the advertised 3A, could it cause an overcurrent fault and collapse the VBUS rail?
[ ... ]
arch/arm64/boot/dts/rockchip/rk3576-anbernic-rg-vita-pro.dts line 1236
Quoted 7 lines
+&spi1 {
arch/arm64/boot/dts/rockchip/rk3576-anbernic-rg-vita-pro.dts new file +6
-0+ num-cs = <1>; + pinctrl-0 = <&spi1m2_pins>, <&spi1m2_csn0>; + status = "okay"; + + /* SPI controlled MCU for joystick and joystick LEDs. */ +};
[Severity: Medium]
The commit message notes the analog joysticks are "controlled via userspace," but the &spi1 node doesn't have a child device node (such as spidev).
Without a child node, the kernel SPI subsystem will not instantiate an SPI device. How will userspace communicate with the joysticks if the device node is missing?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6