On Tue, Jan 12, 2021 at 03:58:30PM +0300, Dmitry Osipenko wrote:
> Use clk_bulk helpers to make code cleaner.
>
> Tested-by: Peter Geis <pgwipeout@xxxxxxxxx>
> Tested-by: Nicolas Chauvet <kwizart@xxxxxxxxx>
> Signed-off-by: Dmitry Osipenko <digetx@xxxxxxxxx>
> ---
> sound/pci/hda/hda_tegra.c | 68 ++++++---------------------------------
> 1 file changed, 9 insertions(+), 59 deletions(-)
Heh... I have a branch samewhere with this same patch. Glad I can cross
that off my list. One thing jumped out at me, see below.
> diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c
> index 70164d1428d4..4c799661c2f6 100644
> --- a/sound/pci/hda/hda_tegra.c
> +++ b/sound/pci/hda/hda_tegra.c
> @@ -70,9 +70,8 @@
> struct hda_tegra {
> struct azx chip;
> struct device *dev;
> - struct clk *hda_clk;
> - struct clk *hda2codec_2x_clk;
> - struct clk *hda2hdmi_clk;
> + struct clk_bulk_data clocks[3];
> + unsigned int nclocks;
> void __iomem *regs;
> struct work_struct probe_work;
> };
> @@ -113,36 +112,6 @@ static void hda_tegra_init(struct hda_tegra *hda)
> writel(v, hda->regs + HDA_IPFS_INTR_MASK);
> }
>
> -static int hda_tegra_enable_clocks(struct hda_tegra *data)
> -{
> - int rc;
> -
> - rc = clk_prepare_enable(data->hda_clk);
> - if (rc)
> - return rc;
> - rc = clk_prepare_enable(data->hda2codec_2x_clk);
> - if (rc)
> - goto disable_hda;
> - rc = clk_prepare_enable(data->hda2hdmi_clk);
> - if (rc)
> - goto disable_codec_2x;
> -
> - return 0;
> -
> -disable_codec_2x:
> - clk_disable_unprepare(data->hda2codec_2x_clk);
> -disable_hda:
> - clk_disable_unprepare(data->hda_clk);
> - return rc;
> -}
> -
> -static void hda_tegra_disable_clocks(struct hda_tegra *data)
> -{
> - clk_disable_unprepare(data->hda2hdmi_clk);
> - clk_disable_unprepare(data->hda2codec_2x_clk);
> - clk_disable_unprepare(data->hda_clk);
> -}
> -
> /*
> * power management
> */
> @@ -186,7 +155,7 @@ static int __maybe_unused hda_tegra_runtime_suspend(struct device *dev)
> azx_stop_chip(chip);
> azx_enter_link_reset(chip);
> }
> - hda_tegra_disable_clocks(hda);
> + clk_bulk_disable_unprepare(hda->nclocks, hda->clocks);
>
> return 0;
> }
> @@ -198,7 +167,7 @@ static int __maybe_unused hda_tegra_runtime_resume(struct device *dev)
> struct hda_tegra *hda = container_of(chip, struct hda_tegra, chip);
> int rc;
>
> - rc = hda_tegra_enable_clocks(hda);
> + rc = clk_bulk_prepare_enable(hda->nclocks, hda->clocks);
> if (rc != 0)
> return rc;
> if (chip && chip->running) {
> @@ -268,29 +237,6 @@ static int hda_tegra_init_chip(struct azx *chip, struct platform_device *pdev)
> return 0;
> }
>
> -static int hda_tegra_init_clk(struct hda_tegra *hda)
> -{
> - struct device *dev = hda->dev;
> -
> - hda->hda_clk = devm_clk_get(dev, "hda");
> - if (IS_ERR(hda->hda_clk)) {
> - dev_err(dev, "failed to get hda clock\n");
> - return PTR_ERR(hda->hda_clk);
> - }
> - hda->hda2codec_2x_clk = devm_clk_get(dev, "hda2codec_2x");
> - if (IS_ERR(hda->hda2codec_2x_clk)) {
> - dev_err(dev, "failed to get hda2codec_2x clock\n");
> - return PTR_ERR(hda->hda2codec_2x_clk);
> - }
> - hda->hda2hdmi_clk = devm_clk_get(dev, "hda2hdmi");
> - if (IS_ERR(hda->hda2hdmi_clk)) {
> - dev_err(dev, "failed to get hda2hdmi clock\n");
> - return PTR_ERR(hda->hda2hdmi_clk);
> - }
> -
> - return 0;
> -}
> -
> static int hda_tegra_first_init(struct azx *chip, struct platform_device *pdev)
> {
> struct hda_tegra *hda = container_of(chip, struct hda_tegra, chip);
> @@ -495,7 +441,11 @@ static int hda_tegra_probe(struct platform_device *pdev)
> return err;
> }
>
> - err = hda_tegra_init_clk(hda);
> + hda->clocks[hda->nclocks++].id = "hda";
> + hda->clocks[hda->nclocks++].id = "hda2hdmi";
> + hda->clocks[hda->nclocks++].id = "hda2codec_2x";
Originally the code did this in this order: "hda", "hda2codec_2x" and
"hda2hdmi". I don't expect the exact order to be very relevant, but was
there any particular reason to change it?
In either case, this should be fine:
Acked-by: Thierry Reding <treding@xxxxxxxxxx>
Attachment:
signature.asc
Description: PGP signature
