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
