Re: [PATCH 3/4] power: supply: axp20x_usb_power: Add support for AXP717

[Oliver Neukum <oneukum@xxxxxxxx>]

On 23.05.24 01:01, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@xxxxxxxxxxx>
>
> Add support for the AXP717. It does USB BC 1.2 detection similar to the
> AXP813, however it is otherwise very different from previous AXP
> devices.
>
> Signed-off-by: Chris Morgan <macromorgan@xxxxxxxxxxx>
> ---

> +static int axp717_usb_init(struct axp20x_usb_power *power)
> +{
> +	int ret;
> +
> +	ret = regmap_update_bits(power->regmap, AXP717_ADC_CH_EN_CONTROL,
> +				 AXP717_ADC_EN_VSYS_VOLT |
> +				 AXP717_ADC_EN_VBUS_VOLT,
> +				 AXP717_ADC_EN_VSYS_VOLT |
> +				 AXP717_ADC_EN_VBUS_VOLT);
> +	if (ret)
> +		return ret;
> +
> +	return 0;

This is a bit silly.

> +}
> +
>   static int axp20x_usb_power_set_voltage_min(struct axp20x_usb_power *power,
>   					    int intval)
>   {
> @@ -307,6 +417,20 @@ static int axp20x_usb_power_set_voltage_min(struct axp20x_usb_power *power,
>   	return -EINVAL;
>   }
>   
> +static int axp717_usb_power_set_voltage_min(struct axp20x_usb_power *power,
> +					    int intval)
> +{
> +	int val;
> +
> +	if (intval < 3880000 || intval > 5080000)

Do you really want raw numbers here?

[..]
> +static int axp717_usb_power_set_property(struct power_supply *psy,
> +					 enum power_supply_property psp,
> +					 const union power_supply_propval *val)
> +{
> +	struct axp20x_usb_power *power = power_supply_get_drvdata(psy);
> +
> +	switch (psp) {
> +	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> +		return axp717_usb_power_set_input_current_limit(power, val->intval);
> +
> +	case POWER_SUPPLY_PROP_VOLTAGE_MIN:
> +		return axp717_usb_power_set_voltage_min(power, val->intval);
> +
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return -EINVAL;

This is also a bit silly.

	Regards
		Oliver