Re: [PATCH v1 18/30] pwm: tegra: Support OPP and core voltage scaling

Thierry Reding
2020-11-10T20:51:08Z

[-- Attachment #1: Type: text/plain, Size: 1723 bytes --]

On Thu, Nov 05, 2020 at 02:44:15AM +0300, Dmitry Osipenko wrote:
[...]
> +static void tegra_pwm_deinit_opp_table(void *data)
> +{
> + struct device *dev = data;
> + struct opp_table *opp_table;
> +
> + opp_table = dev_pm_opp_get_opp_table(dev);
> + dev_pm_opp_of_remove_table(dev);
> + dev_pm_opp_put_regulators(opp_table);
> + dev_pm_opp_put_opp_table(opp_table);
> +}
> +
> +static int devm_tegra_pwm_init_opp_table(struct device *dev)
> +{
> + struct opp_table *opp_table;
> + const char *rname = "core";
> + int err;
> +
> + /* voltage scaling is optional */
> + if (device_property_present(dev, "core-supply"))
> + opp_table = dev_pm_opp_set_regulators(dev, &rname, 1);
> + else
> + opp_table = dev_pm_opp_get_opp_table(dev);
> +
> + if (IS_ERR(opp_table))
> + return dev_err_probe(dev, PTR_ERR(opp_table),
> + "failed to prepare OPP table\n");
> +
> + /*
> + * OPP table presence is optional and we want the set_rate() of OPP
> + * API to work similarly to clk_set_rate() if table is missing in a
> + * device-tree. The add_table() errors out if OPP is missing in DT.
> + */
> + if (device_property_present(dev, "operating-points-v2")) {
> + err = dev_pm_opp_of_add_table(dev);
> + if (err) {
> + dev_err(dev, "failed to add OPP table: %d\n", err);
> + goto put_table;
> + }
> + }
> +
> + err = devm_add_action(dev, tegra_pwm_deinit_opp_table, dev);
> + if (err)
> + goto remove_table;
> +
> + return 0;
> +
> +remove_table:
> + dev_pm_opp_of_remove_table(dev);
> +put_table:
> + dev_pm_opp_put_regulators(opp_table);
> +
> + return err;
> +}

These two functions seem to be heavily boilerplate across all these
drivers. Have you considered splitting these out into separate helpers?

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]