Re: [PATCH v2 2/2] regulator: richtek,rt4801: use existing ena_gpiod feature

[Krzysztof Kozlowski]

On 25/04/2022 07:49, ChiYuan Huang wrote:
>> +static int rt4801_of_parse_cb(struct device_node *np,
>> +			      const struct regulator_desc *desc,
>> +			      struct regulator_config *config)
>> +{
>> +	struct rt4801_priv *priv = config->driver_data;
>> +
>> +	if (priv->enable_gpios) {
>> +		dev_warn(priv->dev, "duplicated enable-gpios property\n");
>> +		return 0;
>> +	}
>> +	config->ena_gpiod = fwnode_gpiod_get_index(of_fwnode_handle(np),
>> +						   "enable-gpios",
> 'enable' only, gpiod API will automatically concat it to 'enable-gpios'.

Right.

>> +						   0,
>> +						   GPIOD_OUT_HIGH |
>> +						   GPIOD_FLAGS_BIT_NONEXCLUSIVE,
>> +						   "rt4801");

(...)

>>  static const struct regulator_ops rt4801_regulator_ops = {
>>  	.list_voltage = regulator_list_voltage_linear,
>>  	.set_voltage_sel = rt4801_set_voltage_sel,
>> @@ -122,6 +168,7 @@ static const struct regulator_desc rt4801_regulator_descs[] = {
>>  		.name = "DSVP",
>>  		.ops = &rt4801_regulator_ops,
>>  		.of_match = of_match_ptr("DSVP"),
>> +		.of_parse_cb = rt4801_of_parse_cb,
>>  		.type = REGULATOR_VOLTAGE,
>>  		.id = DSV_OUT_POS,
>>  		.min_uV = MIN_UV,
>> @@ -135,6 +182,7 @@ static const struct regulator_desc rt4801_regulator_descs[] = {
>>  		.name = "DSVN",
>>  		.ops = &rt4801_regulator_ops,
>>  		.of_match = of_match_ptr("DSVN"),
>> +		.of_parse_cb = rt4801_of_parse_cb,
>>  		.type = REGULATOR_VOLTAGE,
>>  		.id = DSV_OUT_NEG,
>>  		.min_uV = MIN_UV,
> 
> There's one problem.
> If 'ena_gpiod' is specified, it cannot be conexisted with ops
> 'enable/disable/is_enabled' by regulator framework.
> It will cause no one to recover the voltage back.
> You can check the original 'enable' ops.

You mean that regulator voltage is being reset after disabling it?

> 
> How about to only parse gpio in 'of_parse_cb' and put it all into the
> driver data, not to use regulator framework 'ena_gpiod'?

In such case that's the only option. Thanks for the review.


Best regards,
Krzysztof