Re: [PATCH 2/2] usb: misc: usb251xb: configure battery charging from DT

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] usb: misc: usb251xb: missing bat_support check in usb251xb_get_ofdata()
--

commit 7116fd963729fc92bbd453fc4d055b0129eabda3
Author: Jhon Wilker Rodrigues Aquino <jhon.aquino@autotrac.com.br>

usb: misc: usb251xb: configure battery charging from DT

This commit parses the battery-charging-enable port list from the device tree
to configure the battery charging enable register on USB251xB hubs.

> diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c
> index fb0693742f018..166eb541ef161 100644
> --- a/drivers/usb/misc/usb251xb.c
> +++ b/drivers/usb/misc/usb251xb.c
> @@ -577,6 +577,10 @@ static int usb251xb_get_ofdata(struct usb251xb *hub,
>  			      (wchar_t *)hub->serial,
>  			      USB251XB_STRING_BUFSIZE);
>  
> +	hub->bat_charge_en = USB251XB_DEF_BATTERY_CHARGING_ENABLE;
> +	usb251xb_get_ports_field(hub, "battery-charging-enable", data->port_cnt,
> +				 true, &hub->bat_charge_en);

[Severity: Medium]
Should parsing "battery-charging-enable" in usb251xb_get_ofdata() be gated on
data->bat_support?

Commit dd99d106de25 ("usb: misc: usb251xb: Add structure for holding chip
specific values") introduced bat_support in struct usb251xb_data, setting it
to false for usb2517_data and usb2517i_data because the USB2517/USB2517i
variants do not support battery charging:

	static const struct usb251xb_data usb2517_data = {
		.product_id = 0x2517,
		.port_cnt = 7,
		.led_support = true,
		.bat_support = false,
		.product_str = "USB2517",
	};

Earlier in usb251xb_get_ofdata(), the variant-specific LED property checks
data->led_support before applying the configuration:

	if (data->led_support && of_get_property(np, "led-usb-mode", NULL))
		hub->conf_data3 &= ~BIT(1);

Without a similar check on data->bat_support here, bat_support remains unused
and probing a USB2517 or USB2517i node with "battery-charging-enable" present
in the device tree will populate hub->bat_charge_en and write non-zero values
to the reserved register 0xD0 (USB251XB_ADDR_BATTERY_CHARGING_ENABLE) during
usb251xb_connect().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103185514.72144-1-jhonwilker0112@gmail.com?part=2