[alsa-devel] [PATCH v5 1/6] ASoC: Intel: Skylake: Add ssp clock driver
Subhransu S. Prusty
subhransu.s.prusty at intel.com
Mon Dec 18 04:57:16 CET 2017
On Wed, Dec 13, 2017 at 02:30:32PM -0800, Stephen Boyd wrote:
> On 12/11, Sriram Periyasamy wrote:
> > +
> > +static int skl_clk_set_rate(struct clk_hw *hw, unsigned long rate,
> > + unsigned long parent_rate)
> > +{
> > + struct skl_clk *clkdev = to_skl_clk(hw);
> > + struct skl_clk_rate_cfg_table *rcfg;
> > + int clk_type;
> > +
> > + if (!clkdev)
> > + return -ENODEV;
>
> These checks don't make sense. container_of() on clk_hw
> structures returning NULL wouldn't happen.
Sure. will remove.
>
> > +
> > + if (!rate)
> > + return -EINVAL;
> > +
> > + if (__clk_is_enabled(hw->clk) && (clkdev->rate != rate))
>
> Any chance you can directly read the hardware instead of going
> through the framework to find out if the clk is enabled? Seems
No. This involves sending an IPC to DSP to enable clock and interpreting the
return error code. I would like to avoid doing this here in set_rate.
> circular to do it this way.
>
> > + return -EBUSY;
> > +
> > + rcfg = skl_get_rate_cfg(clkdev->pdata->ssp_clks[clkdev->id].rate_cfg,
> > + rate);
> > + if (!rcfg)
> > + return -EINVAL;
> > +
> > + clk_type = skl_get_clk_type(clkdev->id);
> > + if (clk_type < 0)
> > + return clk_type;
> > +
> > + skl_fill_clk_ipc(rcfg, clk_type);
> > + clkdev->rate = rate;
> > +
> > + return 0;
> > +}
> > +
> > +static unsigned long skl_clk_recalc_rate(struct clk_hw *hw,
> > + unsigned long parent_rate)
> > +{
> > + struct skl_clk *clkdev = to_skl_clk(hw);
> > + struct skl_clk_rate_cfg_table *rcfg;
> > + int clk_type;
> > +
> > + if (!clkdev)
> > + return 0;
> > +
> > + if (clkdev->rate)
> > + return clkdev->rate;
>
> Why is the rate being cached? We should always be able to
> calculate the rate based on parent_rate that gets passed to this
> function?
Will check and get back.
Regards,
Subhransu
>
> > +
> > + rcfg = skl_get_rate_cfg(clkdev->pdata->ssp_clks[clkdev->id].rate_cfg,
> > + parent_rate);
> > + if (!rcfg)
> > + return 0;
> > +
> > + clk_type = skl_get_clk_type(clkdev->id);
> > + if (clk_type < 0)
> > + return 0;
> > +
> > + skl_fill_clk_ipc(rcfg, clk_type);
> > + clkdev->rate = rcfg->rate;
> > +
> > + return clkdev->rate;
> > +}
> > +
> > +/* Not supported by clk driver. Implemented to satisfy clk fw */
> > +long skl_clk_round_rate(struct clk_hw *hw, unsigned long rate,
> > + unsigned long *parent_rate)
> > +{
> > + return rate;
> > +}
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
--
More information about the Alsa-devel
mailing list