| Message ID | 1575600535-26877-9-git-send-email-skomatineni@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Move PMC clocks into Tegra PMC driver | expand |
06.12.2019 05:48, Sowjanya Komatineni пишет: > Current ASoC driver uses extern1 as cdev1 clock from Tegra30 onwards > through device tree. > > Actual audio mclk is clk_out_1 and to use PLLA for mclk rate control, > need to clk_out_1_mux parent to extern1 and extern1 parent to PLLA_OUT0. > > Currently Tegra clock driver init sets the parents and enables both > clk_out_1 and extern1 clocks. But these clocks parent and enables should > be controlled by ASoC driver. > > Clock parents can be specified in device tree using assigned-clocks > and assigned-clock-parents. > > To enable audio mclk, both clk_out_1 and extern1 clocks need to be > enabled. > > This patch configures parents for clk_out_1 and extern1 clocks if device > tree does not specify clock parents inorder to support old device tree > and controls mclk using both clk_out_1 and extern1 clocks. > > Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com> > --- > sound/soc/tegra/tegra_asoc_utils.c | 66 ++++++++++++++++++++++++++++++++++++++ > sound/soc/tegra/tegra_asoc_utils.h | 1 + > 2 files changed, 67 insertions(+) > > diff --git a/sound/soc/tegra/tegra_asoc_utils.c b/sound/soc/tegra/tegra_asoc_utils.c > index 536a578e9512..8e3a3740df7c 100644 > --- a/sound/soc/tegra/tegra_asoc_utils.c > +++ b/sound/soc/tegra/tegra_asoc_utils.c > @@ -60,6 +60,7 @@ int tegra_asoc_utils_set_rate(struct tegra_asoc_utils_data *data, int srate, > data->set_mclk = 0; > > clk_disable_unprepare(data->clk_cdev1); > + clk_disable_unprepare(data->clk_extern1); > clk_disable_unprepare(data->clk_pll_a_out0); > clk_disable_unprepare(data->clk_pll_a); > > @@ -89,6 +90,14 @@ int tegra_asoc_utils_set_rate(struct tegra_asoc_utils_data *data, int srate, > return err; > } > > + if (!IS_ERR_OR_NULL(data->clk_extern1)) { > + err = clk_prepare_enable(data->clk_extern1); > + if (err) { > + dev_err(data->dev, "Can't enable extern1: %d\n", err); > + return err; > + } > + } > + > err = clk_prepare_enable(data->clk_cdev1); > if (err) { > dev_err(data->dev, "Can't enable cdev1: %d\n", err); > @@ -109,6 +118,7 @@ int tegra_asoc_utils_set_ac97_rate(struct tegra_asoc_utils_data *data) > int err; > > clk_disable_unprepare(data->clk_cdev1); > + clk_disable_unprepare(data->clk_extern1); > clk_disable_unprepare(data->clk_pll_a_out0); > clk_disable_unprepare(data->clk_pll_a); > > @@ -142,6 +152,14 @@ int tegra_asoc_utils_set_ac97_rate(struct tegra_asoc_utils_data *data) > return err; > } > > + if (!IS_ERR_OR_NULL(data->clk_extern1)) { > + err = clk_prepare_enable(data->clk_extern1); > + if (err) { > + dev_err(data->dev, "Can't enable extern1: %d\n", err); > + return err; > + } > + } Why this is needed given that clk_extern1 is either a child of MCLK or MCLK itself (on T20)? The child clocks are enabled when the parent is enabled. > err = clk_prepare_enable(data->clk_cdev1); > if (err) { > dev_err(data->dev, "Can't enable cdev1: %d\n", err); > @@ -158,6 +176,7 @@ EXPORT_SYMBOL_GPL(tegra_asoc_utils_set_ac97_rate); > int tegra_asoc_utils_init(struct tegra_asoc_utils_data *data, > struct device *dev) > { > + struct clk *clk_out_1_mux; > int ret; > > data->dev = dev; > @@ -196,6 +215,51 @@ int tegra_asoc_utils_init(struct tegra_asoc_utils_data *data, > goto err_put_pll_a_out0; > } In a previous patch you added fallback to EXTPERIPH when clk_get(MCLK) fails. This will work perfectly fine for the older kernels which have all clocks in the same single CaR driver, but this may not work that great for the newer kernels because PMC driver isn't registered early during boot and thus it is possible to get a legit -EPROBE_DEFER which shouldn't be ignored. In other words, you need to add into this patch a check for the error code returned by clk_get(MCLK) and fallback only for -EINVAL. > + /* > + * If clock parents are not set in DT, configure here to use clk_out_1 > + * as mclk and extern1 as parent for Tegra30 and higher. > + */ > + if (!of_find_property(dev->of_node, "assigned-clock-parents", NULL) && > + data->soc > TEGRA_ASOC_UTILS_SOC_TEGRA20) { > + data->clk_extern1 = clk_get_sys("clk_out_1", "extern1"); > + if (IS_ERR(data->clk_extern1)) { > + dev_err(data->dev, "Can't retrieve clk extern1\n"); > + ret = PTR_ERR(data->clk_extern1); > + goto err_put_cdev1; > + } > + > + ret = clk_set_parent(data->clk_extern1, data->clk_pll_a_out0); > + if (ret < 0) { > + dev_err(data->dev, > + "Set parent failed for clk extern1: %d\n", > + ret); > + goto err_put_cdev1; > + } > + > + clk_out_1_mux = clk_get_sys(NULL, "clk_out_1_mux"); Note1: clk_get(dev, "clk_out_1_mux") should work here by letting clk core to fall back to the clk_get_sys() by itself. Either way should be good. Note2: devm_clk_get() could be used everywhere here. Maybe it won't hurt to convert tegra_asoc_utils to use managed resources to keep code a bit cleaner. It should be a separate patch. > + if (IS_ERR(clk_out_1_mux)) { > + dev_err(data->dev, "Can't retrieve clk clk_out_1_mux\n"); > + ret = PTR_ERR(clk_out_1_mux); > + goto err_put_cdev1; > + } > + > + ret = clk_set_parent(clk_out_1_mux, data->clk_extern1); > + if (ret < 0) { > + dev_err(data->dev, > + "Set parent failed for clk_out_1_mux: %d\n", > + ret); > + clk_put(clk_out_1_mux); > + goto err_put_cdev1; > + } clk_put(clk_cdev1); > + data->clk_cdev1 = clk_get_sys(NULL, "clk_out_1"); > + if (IS_ERR(data->clk_cdev1)) { > + dev_err(data->dev, "Can't retrieve clk clk_out_1\n"); > + ret = PTR_ERR(data->clk_cdev1); > + goto err_put_cdev1; goto err_put_pll_a_out0; > + } > + } > + > ret = tegra_asoc_utils_set_rate(data, 44100, 256 * 44100); > if (ret) > goto err_put_cdev1; > @@ -215,6 +279,8 @@ EXPORT_SYMBOL_GPL(tegra_asoc_utils_init); > > void tegra_asoc_utils_fini(struct tegra_asoc_utils_data *data) > { > + if (!IS_ERR_OR_NULL(data->clk_extern1)) > + clk_put(data->clk_extern1); > clk_put(data->clk_cdev1); > clk_put(data->clk_pll_a_out0); > clk_put(data->clk_pll_a); > diff --git a/sound/soc/tegra/tegra_asoc_utils.h b/sound/soc/tegra/tegra_asoc_utils.h > index 0c13818dee75..5f2b96478caf 100644 > --- a/sound/soc/tegra/tegra_asoc_utils.h > +++ b/sound/soc/tegra/tegra_asoc_utils.h > @@ -25,6 +25,7 @@ struct tegra_asoc_utils_data { > struct clk *clk_pll_a; > struct clk *clk_pll_a_out0; > struct clk *clk_cdev1; > + struct clk *clk_extern1; > int set_baseclock; > int set_mclk; > }; >
07.12.2019 22:20, Sowjanya Komatineni пишет: > > On 12/7/19 6:58 AM, Dmitry Osipenko wrote: >> 06.12.2019 05:48, Sowjanya Komatineni пишет: >>> Current ASoC driver uses extern1 as cdev1 clock from Tegra30 onwards >>> through device tree. >>> >>> Actual audio mclk is clk_out_1 and to use PLLA for mclk rate control, >>> need to clk_out_1_mux parent to extern1 and extern1 parent to PLLA_OUT0. >>> >>> Currently Tegra clock driver init sets the parents and enables both >>> clk_out_1 and extern1 clocks. But these clocks parent and enables should >>> be controlled by ASoC driver. >>> >>> Clock parents can be specified in device tree using assigned-clocks >>> and assigned-clock-parents. >>> >>> To enable audio mclk, both clk_out_1 and extern1 clocks need to be >>> enabled. >>> >>> This patch configures parents for clk_out_1 and extern1 clocks if device >>> tree does not specify clock parents inorder to support old device tree >>> and controls mclk using both clk_out_1 and extern1 clocks. >>> >>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com> >>> --- >>> sound/soc/tegra/tegra_asoc_utils.c | 66 >>> ++++++++++++++++++++++++++++++++++++++ >>> sound/soc/tegra/tegra_asoc_utils.h | 1 + >>> 2 files changed, 67 insertions(+) >>> >>> diff --git a/sound/soc/tegra/tegra_asoc_utils.c >>> b/sound/soc/tegra/tegra_asoc_utils.c >>> index 536a578e9512..8e3a3740df7c 100644 >>> --- a/sound/soc/tegra/tegra_asoc_utils.c >>> +++ b/sound/soc/tegra/tegra_asoc_utils.c >>> @@ -60,6 +60,7 @@ int tegra_asoc_utils_set_rate(struct >>> tegra_asoc_utils_data *data, int srate, >>> data->set_mclk = 0; >>> clk_disable_unprepare(data->clk_cdev1); >>> + clk_disable_unprepare(data->clk_extern1); >>> clk_disable_unprepare(data->clk_pll_a_out0); >>> clk_disable_unprepare(data->clk_pll_a); >>> @@ -89,6 +90,14 @@ int tegra_asoc_utils_set_rate(struct >>> tegra_asoc_utils_data *data, int srate, >>> return err; >>> } >>> + if (!IS_ERR_OR_NULL(data->clk_extern1)) { >>> + err = clk_prepare_enable(data->clk_extern1); >>> + if (err) { >>> + dev_err(data->dev, "Can't enable extern1: %d\n", err); >>> + return err; >>> + } >>> + } >>> + >>> err = clk_prepare_enable(data->clk_cdev1); >>> if (err) { >>> dev_err(data->dev, "Can't enable cdev1: %d\n", err); >>> @@ -109,6 +118,7 @@ int tegra_asoc_utils_set_ac97_rate(struct >>> tegra_asoc_utils_data *data) >>> int err; >>> clk_disable_unprepare(data->clk_cdev1); >>> + clk_disable_unprepare(data->clk_extern1); >>> clk_disable_unprepare(data->clk_pll_a_out0); >>> clk_disable_unprepare(data->clk_pll_a); >>> @@ -142,6 +152,14 @@ int tegra_asoc_utils_set_ac97_rate(struct >>> tegra_asoc_utils_data *data) >>> return err; >>> } >>> + if (!IS_ERR_OR_NULL(data->clk_extern1)) { >>> + err = clk_prepare_enable(data->clk_extern1); >>> + if (err) { >>> + dev_err(data->dev, "Can't enable extern1: %d\n", err); >>> + return err; >>> + } >>> + } >> Why this is needed given that clk_extern1 is either a child of MCLK or >> MCLK itself (on T20)? The child clocks are enabled when the parent is >> enabled. > > For T30 and later, clk_extern1 is one of the source for clk_out_1_mux. > clk_extern1 is in CAR and it has its own gate and mux. > > As audio mclk related clocks (clk_out_1, clk_out_1_mux, and extern1) are > moved into ASoC driver from clock driver > > need to enable extern1 gate as well along with clk_out1 for T30 through > T210. > > Just FYI, extern1 enable here happens only when data->clk_extern1 is > available which is for T30 onwards. clk_out_1 is the parent of extern1, thus extern1 is enabled by the clk core whenever clk_out_1 is enabled because data->clk_cdev1=clk_out_1. An I missing something? [snip]
10.12.2019 02:05, Sowjanya Komatineni пишет: > > On 12/9/19 12:06 PM, Dmitry Osipenko wrote: >> 07.12.2019 22:20, Sowjanya Komatineni пишет: >>> On 12/7/19 6:58 AM, Dmitry Osipenko wrote: >>>> 06.12.2019 05:48, Sowjanya Komatineni пишет: >>>>> Current ASoC driver uses extern1 as cdev1 clock from Tegra30 onwards >>>>> through device tree. >>>>> >>>>> Actual audio mclk is clk_out_1 and to use PLLA for mclk rate control, >>>>> need to clk_out_1_mux parent to extern1 and extern1 parent to >>>>> PLLA_OUT0. >>>>> >>>>> Currently Tegra clock driver init sets the parents and enables both >>>>> clk_out_1 and extern1 clocks. But these clocks parent and enables >>>>> should >>>>> be controlled by ASoC driver. >>>>> >>>>> Clock parents can be specified in device tree using assigned-clocks >>>>> and assigned-clock-parents. >>>>> >>>>> To enable audio mclk, both clk_out_1 and extern1 clocks need to be >>>>> enabled. >>>>> >>>>> This patch configures parents for clk_out_1 and extern1 clocks if >>>>> device >>>>> tree does not specify clock parents inorder to support old device tree >>>>> and controls mclk using both clk_out_1 and extern1 clocks. >>>>> >>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com> >>>>> --- >>>>> sound/soc/tegra/tegra_asoc_utils.c | 66 >>>>> ++++++++++++++++++++++++++++++++++++++ >>>>> sound/soc/tegra/tegra_asoc_utils.h | 1 + >>>>> 2 files changed, 67 insertions(+) >>>>> >>>>> diff --git a/sound/soc/tegra/tegra_asoc_utils.c >>>>> b/sound/soc/tegra/tegra_asoc_utils.c >>>>> index 536a578e9512..8e3a3740df7c 100644 >>>>> --- a/sound/soc/tegra/tegra_asoc_utils.c >>>>> +++ b/sound/soc/tegra/tegra_asoc_utils.c >>>>> @@ -60,6 +60,7 @@ int tegra_asoc_utils_set_rate(struct >>>>> tegra_asoc_utils_data *data, int srate, >>>>> data->set_mclk = 0; >>>>> clk_disable_unprepare(data->clk_cdev1); >>>>> + clk_disable_unprepare(data->clk_extern1); >>>>> clk_disable_unprepare(data->clk_pll_a_out0); >>>>> clk_disable_unprepare(data->clk_pll_a); >>>>> @@ -89,6 +90,14 @@ int tegra_asoc_utils_set_rate(struct >>>>> tegra_asoc_utils_data *data, int srate, >>>>> return err; >>>>> } >>>>> + if (!IS_ERR_OR_NULL(data->clk_extern1)) { >>>>> + err = clk_prepare_enable(data->clk_extern1); >>>>> + if (err) { >>>>> + dev_err(data->dev, "Can't enable extern1: %d\n", err); >>>>> + return err; >>>>> + } >>>>> + } >>>>> + >>>>> err = clk_prepare_enable(data->clk_cdev1); >>>>> if (err) { >>>>> dev_err(data->dev, "Can't enable cdev1: %d\n", err); >>>>> @@ -109,6 +118,7 @@ int tegra_asoc_utils_set_ac97_rate(struct >>>>> tegra_asoc_utils_data *data) >>>>> int err; >>>>> clk_disable_unprepare(data->clk_cdev1); >>>>> + clk_disable_unprepare(data->clk_extern1); >>>>> clk_disable_unprepare(data->clk_pll_a_out0); >>>>> clk_disable_unprepare(data->clk_pll_a); >>>>> @@ -142,6 +152,14 @@ int tegra_asoc_utils_set_ac97_rate(struct >>>>> tegra_asoc_utils_data *data) >>>>> return err; >>>>> } >>>>> + if (!IS_ERR_OR_NULL(data->clk_extern1)) { >>>>> + err = clk_prepare_enable(data->clk_extern1); >>>>> + if (err) { >>>>> + dev_err(data->dev, "Can't enable extern1: %d\n", err); >>>>> + return err; >>>>> + } >>>>> + } >>>> Why this is needed given that clk_extern1 is either a child of MCLK or >>>> MCLK itself (on T20)? The child clocks are enabled when the parent is >>>> enabled. >>> For T30 and later, clk_extern1 is one of the source for clk_out_1_mux. >>> clk_extern1 is in CAR and it has its own gate and mux. >>> >>> As audio mclk related clocks (clk_out_1, clk_out_1_mux, and extern1) are >>> moved into ASoC driver from clock driver >>> >>> need to enable extern1 gate as well along with clk_out1 for T30 through >>> T210. >>> >>> Just FYI, extern1 enable here happens only when data->clk_extern1 is >>> available which is for T30 onwards. >> clk_out_1 is the parent of extern1, thus extern1 is enabled by the clk >> core whenever clk_out_1 is enabled because data->clk_cdev1=clk_out_1. An >> I missing something? >> >> [snip] > extern1 is the parent for clk_out_1. explained extern1 clock path to > clk_out in reply to your comment in other patch of this series. Right, I meant extern1 the parent of clk_out_1, sorry for the confusion. So when clk_out_1 (child) is enabled, extern1 (parent) is enabled as well. I'll take a closer look at the other email tomorrow.
17.12.2019 04:29, Sowjanya Komatineni пишет: > > On 12/7/19 11:20 AM, Sowjanya Komatineni wrote: >> >> On 12/7/19 6:58 AM, Dmitry Osipenko wrote: >>> 06.12.2019 05:48, Sowjanya Komatineni пишет: >>>> Current ASoC driver uses extern1 as cdev1 clock from Tegra30 onwards >>>> through device tree. >>>> >>>> Actual audio mclk is clk_out_1 and to use PLLA for mclk rate control, >>>> need to clk_out_1_mux parent to extern1 and extern1 parent to >>>> PLLA_OUT0. >>>> >>>> Currently Tegra clock driver init sets the parents and enables both >>>> clk_out_1 and extern1 clocks. But these clocks parent and enables >>>> should >>>> be controlled by ASoC driver. >>>> >>>> Clock parents can be specified in device tree using assigned-clocks >>>> and assigned-clock-parents. >>>> >>>> To enable audio mclk, both clk_out_1 and extern1 clocks need to be >>>> enabled. >>>> >>>> This patch configures parents for clk_out_1 and extern1 clocks if >>>> device >>>> tree does not specify clock parents inorder to support old device tree >>>> and controls mclk using both clk_out_1 and extern1 clocks. >>>> >>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com> >>>> --- >>>> sound/soc/tegra/tegra_asoc_utils.c | 66 >>>> ++++++++++++++++++++++++++++++++++++++ >>>> sound/soc/tegra/tegra_asoc_utils.h | 1 + >>>> 2 files changed, 67 insertions(+) >>>> >>>> diff --git a/sound/soc/tegra/tegra_asoc_utils.c >>>> b/sound/soc/tegra/tegra_asoc_utils.c >>>> index 536a578e9512..8e3a3740df7c 100644 >>>> --- a/sound/soc/tegra/tegra_asoc_utils.c >>>> +++ b/sound/soc/tegra/tegra_asoc_utils.c >>>> @@ -60,6 +60,7 @@ int tegra_asoc_utils_set_rate(struct >>>> tegra_asoc_utils_data *data, int srate, >>>> data->set_mclk = 0; >>>> clk_disable_unprepare(data->clk_cdev1); >>>> + clk_disable_unprepare(data->clk_extern1); >>>> clk_disable_unprepare(data->clk_pll_a_out0); >>>> clk_disable_unprepare(data->clk_pll_a); >>>> @@ -89,6 +90,14 @@ int tegra_asoc_utils_set_rate(struct >>>> tegra_asoc_utils_data *data, int srate, >>>> return err; >>>> } >>>> + if (!IS_ERR_OR_NULL(data->clk_extern1)) { >>>> + err = clk_prepare_enable(data->clk_extern1); >>>> + if (err) { >>>> + dev_err(data->dev, "Can't enable extern1: %d\n", err); >>>> + return err; >>>> + } >>>> + } >>>> + >>>> err = clk_prepare_enable(data->clk_cdev1); >>>> if (err) { >>>> dev_err(data->dev, "Can't enable cdev1: %d\n", err); >>>> @@ -109,6 +118,7 @@ int tegra_asoc_utils_set_ac97_rate(struct >>>> tegra_asoc_utils_data *data) >>>> int err; >>>> clk_disable_unprepare(data->clk_cdev1); >>>> + clk_disable_unprepare(data->clk_extern1); >>>> clk_disable_unprepare(data->clk_pll_a_out0); >>>> clk_disable_unprepare(data->clk_pll_a); >>>> @@ -142,6 +152,14 @@ int tegra_asoc_utils_set_ac97_rate(struct >>>> tegra_asoc_utils_data *data) >>>> return err; >>>> } >>>> + if (!IS_ERR_OR_NULL(data->clk_extern1)) { >>>> + err = clk_prepare_enable(data->clk_extern1); >>>> + if (err) { >>>> + dev_err(data->dev, "Can't enable extern1: %d\n", err); >>>> + return err; >>>> + } >>>> + } >>> Why this is needed given that clk_extern1 is either a child of MCLK or >>> MCLK itself (on T20)? The child clocks are enabled when the parent is >>> enabled. >> >> For T30 and later, clk_extern1 is one of the source for clk_out_1_mux. >> clk_extern1 is in CAR and it has its own gate and mux. >> >> As audio mclk related clocks (clk_out_1, clk_out_1_mux, and extern1) >> are moved into ASoC driver from clock driver >> >> need to enable extern1 gate as well along with clk_out1 for T30 >> through T210. >> >> Just FYI, extern1 enable here happens only when data->clk_extern1 is >> available which is for T30 onwards. >> >>>> err = clk_prepare_enable(data->clk_cdev1); >>>> if (err) { >>>> dev_err(data->dev, "Can't enable cdev1: %d\n", err); >>>> @@ -158,6 +176,7 @@ EXPORT_SYMBOL_GPL(tegra_asoc_utils_set_ac97_rate); >>>> int tegra_asoc_utils_init(struct tegra_asoc_utils_data *data, >>>> struct device *dev) >>>> { >>>> + struct clk *clk_out_1_mux; >>>> int ret; >>>> data->dev = dev; >>>> @@ -196,6 +215,51 @@ int tegra_asoc_utils_init(struct >>>> tegra_asoc_utils_data *data, >>>> goto err_put_pll_a_out0; >>>> } >>> In a previous patch you added fallback to EXTPERIPH when clk_get(MCLK) >>> fails. This will work perfectly fine for the older kernels which have >>> all clocks in the same single CaR driver, but this may not work that >>> great for the newer kernels because PMC driver isn't registered early >>> during boot and thus it is possible to get a legit -EPROBE_DEFER which >>> shouldn't be ignored. In other words, you need to add into this patch a >>> check for the error code returned by clk_get(MCLK) and fallback only for >>> -EINVAL. >> yeah right, will add check in next version. >>>> + /* >>>> + * If clock parents are not set in DT, configure here to use >>>> clk_out_1 >>>> + * as mclk and extern1 as parent for Tegra30 and higher. >>>> + */ >>>> + if (!of_find_property(dev->of_node, "assigned-clock-parents", >>>> NULL) && >>>> + data->soc > TEGRA_ASOC_UTILS_SOC_TEGRA20) { >>>> + data->clk_extern1 = clk_get_sys("clk_out_1", "extern1"); >>>> + if (IS_ERR(data->clk_extern1)) { >>>> + dev_err(data->dev, "Can't retrieve clk extern1\n"); >>>> + ret = PTR_ERR(data->clk_extern1); >>>> + goto err_put_cdev1; >>>> + } >>>> + >>>> + ret = clk_set_parent(data->clk_extern1, data->clk_pll_a_out0); >>>> + if (ret < 0) { >>>> + dev_err(data->dev, >>>> + "Set parent failed for clk extern1: %d\n", >>>> + ret); >>>> + goto err_put_cdev1; >>>> + } >>>> + >>>> + clk_out_1_mux = clk_get_sys(NULL, "clk_out_1_mux"); >>> Note1: clk_get(dev, "clk_out_1_mux") should work here by letting clk >>> core to fall back to the clk_get_sys() by itself. Either way should >>> be good. > > clk_get uses device rather and dev_id will be name of this device and > when clk_get fall back to __clk_get_sys() it still will use dev id of > this device rather than actual dev_id that pmc clocks are added to the > lookup. So clk_get_sys() seems to be correct to use as we can specify > exact dev_id and con_id. It should be better to use something "resource managed", thus devm_clk_get() should be a better choice. > Also, clk_find retrieves clock from lookup only when it finds matching > clock with both dev_id and con_id as pmc clocks are registered with both > dev_id and con_id. > > I see existing clock driver adds both extern and pmc clocks (clk_out) to > lookup with same dev_id of clk_out_1/2/3 and con_id of extern1/2/3 and > with this always extern clock will be retrieved and this is probably > because old DT and audio driver always uses extern1 rather than actual > clk_out_1 > > But this need to be fixed now as we changed to use clk_out directly > rather than extern (even for other pmc clocks) to match actual hw design. > > Will fix this as well to register pmc clocks using con_id as > clk_out_1/2/3 in pmc driver and extern clocks using con_id of > extern1/2/3 with dev_id being NULL so we can retrieve these clocks by > just using con_id only using clk_get_sys as we switched to use clk_out_1 > directly as pmc clock rather than extern from DT and no longer need to > pair pmc clocks to extern clocks. I'm not sure it's worth the effort to care about con_ids if implicit fallback to clk_get_sys(NULL, "...") does the right thing for the audio driver. IIRC, CCF uses variant of matching clocks by names, although I'm not sure whether that applies to older stable kernels. [snip]
17.12.2019 19:12, Sowjanya Komatineni пишет: > > On 12/17/19 7:36 AM, Dmitry Osipenko wrote: >> 17.12.2019 04:29, Sowjanya Komatineni пишет: >>> On 12/7/19 11:20 AM, Sowjanya Komatineni wrote: >>>> On 12/7/19 6:58 AM, Dmitry Osipenko wrote: >>>>> 06.12.2019 05:48, Sowjanya Komatineni пишет: >>>>>> Current ASoC driver uses extern1 as cdev1 clock from Tegra30 onwards >>>>>> through device tree. >>>>>> >>>>>> Actual audio mclk is clk_out_1 and to use PLLA for mclk rate control, >>>>>> need to clk_out_1_mux parent to extern1 and extern1 parent to >>>>>> PLLA_OUT0. >>>>>> >>>>>> Currently Tegra clock driver init sets the parents and enables both >>>>>> clk_out_1 and extern1 clocks. But these clocks parent and enables >>>>>> should >>>>>> be controlled by ASoC driver. >>>>>> >>>>>> Clock parents can be specified in device tree using assigned-clocks >>>>>> and assigned-clock-parents. >>>>>> >>>>>> To enable audio mclk, both clk_out_1 and extern1 clocks need to be >>>>>> enabled. >>>>>> >>>>>> This patch configures parents for clk_out_1 and extern1 clocks if >>>>>> device >>>>>> tree does not specify clock parents inorder to support old device >>>>>> tree >>>>>> and controls mclk using both clk_out_1 and extern1 clocks. >>>>>> >>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com> >>>>>> --- >>>>>> sound/soc/tegra/tegra_asoc_utils.c | 66 >>>>>> ++++++++++++++++++++++++++++++++++++++ >>>>>> sound/soc/tegra/tegra_asoc_utils.h | 1 + >>>>>> 2 files changed, 67 insertions(+) >>>>>> >>>>>> diff --git a/sound/soc/tegra/tegra_asoc_utils.c >>>>>> b/sound/soc/tegra/tegra_asoc_utils.c >>>>>> index 536a578e9512..8e3a3740df7c 100644 >>>>>> --- a/sound/soc/tegra/tegra_asoc_utils.c >>>>>> +++ b/sound/soc/tegra/tegra_asoc_utils.c >>>>>> @@ -60,6 +60,7 @@ int tegra_asoc_utils_set_rate(struct >>>>>> tegra_asoc_utils_data *data, int srate, >>>>>> data->set_mclk = 0; >>>>>> clk_disable_unprepare(data->clk_cdev1); >>>>>> + clk_disable_unprepare(data->clk_extern1); >>>>>> clk_disable_unprepare(data->clk_pll_a_out0); >>>>>> clk_disable_unprepare(data->clk_pll_a); >>>>>> @@ -89,6 +90,14 @@ int tegra_asoc_utils_set_rate(struct >>>>>> tegra_asoc_utils_data *data, int srate, >>>>>> return err; >>>>>> } >>>>>> + if (!IS_ERR_OR_NULL(data->clk_extern1)) { >>>>>> + err = clk_prepare_enable(data->clk_extern1); >>>>>> + if (err) { >>>>>> + dev_err(data->dev, "Can't enable extern1: %d\n", err); >>>>>> + return err; >>>>>> + } >>>>>> + } >>>>>> + >>>>>> err = clk_prepare_enable(data->clk_cdev1); >>>>>> if (err) { >>>>>> dev_err(data->dev, "Can't enable cdev1: %d\n", err); >>>>>> @@ -109,6 +118,7 @@ int tegra_asoc_utils_set_ac97_rate(struct >>>>>> tegra_asoc_utils_data *data) >>>>>> int err; >>>>>> clk_disable_unprepare(data->clk_cdev1); >>>>>> + clk_disable_unprepare(data->clk_extern1); >>>>>> clk_disable_unprepare(data->clk_pll_a_out0); >>>>>> clk_disable_unprepare(data->clk_pll_a); >>>>>> @@ -142,6 +152,14 @@ int tegra_asoc_utils_set_ac97_rate(struct >>>>>> tegra_asoc_utils_data *data) >>>>>> return err; >>>>>> } >>>>>> + if (!IS_ERR_OR_NULL(data->clk_extern1)) { >>>>>> + err = clk_prepare_enable(data->clk_extern1); >>>>>> + if (err) { >>>>>> + dev_err(data->dev, "Can't enable extern1: %d\n", err); >>>>>> + return err; >>>>>> + } >>>>>> + } >>>>> Why this is needed given that clk_extern1 is either a child of MCLK or >>>>> MCLK itself (on T20)? The child clocks are enabled when the parent is >>>>> enabled. >>>> For T30 and later, clk_extern1 is one of the source for clk_out_1_mux. >>>> clk_extern1 is in CAR and it has its own gate and mux. >>>> >>>> As audio mclk related clocks (clk_out_1, clk_out_1_mux, and extern1) >>>> are moved into ASoC driver from clock driver >>>> >>>> need to enable extern1 gate as well along with clk_out1 for T30 >>>> through T210. >>>> >>>> Just FYI, extern1 enable here happens only when data->clk_extern1 is >>>> available which is for T30 onwards. >>>> >>>>>> err = clk_prepare_enable(data->clk_cdev1); >>>>>> if (err) { >>>>>> dev_err(data->dev, "Can't enable cdev1: %d\n", err); >>>>>> @@ -158,6 +176,7 @@ >>>>>> EXPORT_SYMBOL_GPL(tegra_asoc_utils_set_ac97_rate); >>>>>> int tegra_asoc_utils_init(struct tegra_asoc_utils_data *data, >>>>>> struct device *dev) >>>>>> { >>>>>> + struct clk *clk_out_1_mux; >>>>>> int ret; >>>>>> data->dev = dev; >>>>>> @@ -196,6 +215,51 @@ int tegra_asoc_utils_init(struct >>>>>> tegra_asoc_utils_data *data, >>>>>> goto err_put_pll_a_out0; >>>>>> } >>>>> In a previous patch you added fallback to EXTPERIPH when clk_get(MCLK) >>>>> fails. This will work perfectly fine for the older kernels which have >>>>> all clocks in the same single CaR driver, but this may not work that >>>>> great for the newer kernels because PMC driver isn't registered early >>>>> during boot and thus it is possible to get a legit -EPROBE_DEFER which >>>>> shouldn't be ignored. In other words, you need to add into this >>>>> patch a >>>>> check for the error code returned by clk_get(MCLK) and fallback >>>>> only for >>>>> -EINVAL. >>>> yeah right, will add check in next version. >>>>>> + /* >>>>>> + * If clock parents are not set in DT, configure here to use >>>>>> clk_out_1 >>>>>> + * as mclk and extern1 as parent for Tegra30 and higher. >>>>>> + */ >>>>>> + if (!of_find_property(dev->of_node, "assigned-clock-parents", >>>>>> NULL) && >>>>>> + data->soc > TEGRA_ASOC_UTILS_SOC_TEGRA20) { >>>>>> + data->clk_extern1 = clk_get_sys("clk_out_1", "extern1"); >>>>>> + if (IS_ERR(data->clk_extern1)) { >>>>>> + dev_err(data->dev, "Can't retrieve clk extern1\n"); >>>>>> + ret = PTR_ERR(data->clk_extern1); >>>>>> + goto err_put_cdev1; >>>>>> + } >>>>>> + >>>>>> + ret = clk_set_parent(data->clk_extern1, >>>>>> data->clk_pll_a_out0); >>>>>> + if (ret < 0) { >>>>>> + dev_err(data->dev, >>>>>> + "Set parent failed for clk extern1: %d\n", >>>>>> + ret); >>>>>> + goto err_put_cdev1; >>>>>> + } >>>>>> + >>>>>> + clk_out_1_mux = clk_get_sys(NULL, "clk_out_1_mux"); >>>>> Note1: clk_get(dev, "clk_out_1_mux") should work here by letting clk >>>>> core to fall back to the clk_get_sys() by itself. Either way should >>>>> be good. >>> clk_get uses device rather and dev_id will be name of this device and >>> when clk_get fall back to __clk_get_sys() it still will use dev id of >>> this device rather than actual dev_id that pmc clocks are added to the >>> lookup. So clk_get_sys() seems to be correct to use as we can specify >>> exact dev_id and con_id. >> It should be better to use something "resource managed", thus >> devm_clk_get() should be a better choice. >> >>> Also, clk_find retrieves clock from lookup only when it finds matching >>> clock with both dev_id and con_id as pmc clocks are registered with both >>> dev_id and con_id. >>> >>> I see existing clock driver adds both extern and pmc clocks (clk_out) to >>> lookup with same dev_id of clk_out_1/2/3 and con_id of extern1/2/3 and >>> with this always extern clock will be retrieved and this is probably >>> because old DT and audio driver always uses extern1 rather than actual >>> clk_out_1 >>> >>> But this need to be fixed now as we changed to use clk_out directly >>> rather than extern (even for other pmc clocks) to match actual hw >>> design. >>> >>> Will fix this as well to register pmc clocks using con_id as >>> clk_out_1/2/3 in pmc driver and extern clocks using con_id of >>> extern1/2/3 with dev_id being NULL so we can retrieve these clocks by >>> just using con_id only using clk_get_sys as we switched to use clk_out_1 >>> directly as pmc clock rather than extern from DT and no longer need to >>> pair pmc clocks to extern clocks. >> I'm not sure it's worth the effort to care about con_ids if implicit >> fallback to clk_get_sys(NULL, "...") does the right thing for the audio >> driver. >> >> IIRC, CCF uses variant of matching clocks by names, although I'm not >> sure whether that applies to older stable kernels. >> >> [snip] > > Current clock driver adds EXTERN clock to lookup with dev_id as > clk_out_1/2/3 and con_id as extern_1/2/3 > > With this we can retrieve clock from lookup only with clk_get_sys where > we can pass dev_id as clk_out_1/2/3 and con_id as extern_1/2/3. > > We cant use devm_clk_get() or clk_get() for retrieving clocks from > lookup by passing device object from sound driver as dev_id will be diff > and clk_find will return NULL. Have you actually tried to test that it fails? I think it's a false assumption. > But with the fix of having dev_id as NULL and use only con_id to add to > lookup, we can use resource managed APIs devm_clk_get. > > So was saying will fix this in clock driver as part of removing > clk_out_1/2/3 ids and pmc init from clock driver so we can use > devm_clk_get API in audio driver. > >>
17.12.2019 19:39, Sowjanya Komatineni пишет: > > On 12/17/19 8:16 AM, Dmitry Osipenko wrote: >> 17.12.2019 19:12, Sowjanya Komatineni пишет: >>> On 12/17/19 7:36 AM, Dmitry Osipenko wrote: >>>> 17.12.2019 04:29, Sowjanya Komatineni пишет: >>>>> On 12/7/19 11:20 AM, Sowjanya Komatineni wrote: >>>>>> On 12/7/19 6:58 AM, Dmitry Osipenko wrote: >>>>>>> 06.12.2019 05:48, Sowjanya Komatineni пишет: >>>>>>>> Current ASoC driver uses extern1 as cdev1 clock from Tegra30 >>>>>>>> onwards >>>>>>>> through device tree. >>>>>>>> >>>>>>>> Actual audio mclk is clk_out_1 and to use PLLA for mclk rate >>>>>>>> control, >>>>>>>> need to clk_out_1_mux parent to extern1 and extern1 parent to >>>>>>>> PLLA_OUT0. >>>>>>>> >>>>>>>> Currently Tegra clock driver init sets the parents and enables both >>>>>>>> clk_out_1 and extern1 clocks. But these clocks parent and enables >>>>>>>> should >>>>>>>> be controlled by ASoC driver. >>>>>>>> >>>>>>>> Clock parents can be specified in device tree using assigned-clocks >>>>>>>> and assigned-clock-parents. >>>>>>>> >>>>>>>> To enable audio mclk, both clk_out_1 and extern1 clocks need to be >>>>>>>> enabled. >>>>>>>> >>>>>>>> This patch configures parents for clk_out_1 and extern1 clocks if >>>>>>>> device >>>>>>>> tree does not specify clock parents inorder to support old device >>>>>>>> tree >>>>>>>> and controls mclk using both clk_out_1 and extern1 clocks. >>>>>>>> >>>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com> >>>>>>>> --- >>>>>>>> sound/soc/tegra/tegra_asoc_utils.c | 66 >>>>>>>> ++++++++++++++++++++++++++++++++++++++ >>>>>>>> sound/soc/tegra/tegra_asoc_utils.h | 1 + >>>>>>>> 2 files changed, 67 insertions(+) >>>>>>>> >>>>>>>> diff --git a/sound/soc/tegra/tegra_asoc_utils.c >>>>>>>> b/sound/soc/tegra/tegra_asoc_utils.c >>>>>>>> index 536a578e9512..8e3a3740df7c 100644 >>>>>>>> --- a/sound/soc/tegra/tegra_asoc_utils.c >>>>>>>> +++ b/sound/soc/tegra/tegra_asoc_utils.c >>>>>>>> @@ -60,6 +60,7 @@ int tegra_asoc_utils_set_rate(struct >>>>>>>> tegra_asoc_utils_data *data, int srate, >>>>>>>> data->set_mclk = 0; >>>>>>>> clk_disable_unprepare(data->clk_cdev1); >>>>>>>> + clk_disable_unprepare(data->clk_extern1); >>>>>>>> clk_disable_unprepare(data->clk_pll_a_out0); >>>>>>>> clk_disable_unprepare(data->clk_pll_a); >>>>>>>> @@ -89,6 +90,14 @@ int tegra_asoc_utils_set_rate(struct >>>>>>>> tegra_asoc_utils_data *data, int srate, >>>>>>>> return err; >>>>>>>> } >>>>>>>> + if (!IS_ERR_OR_NULL(data->clk_extern1)) { >>>>>>>> + err = clk_prepare_enable(data->clk_extern1); >>>>>>>> + if (err) { >>>>>>>> + dev_err(data->dev, "Can't enable extern1: %d\n", err); >>>>>>>> + return err; >>>>>>>> + } >>>>>>>> + } >>>>>>>> + >>>>>>>> err = clk_prepare_enable(data->clk_cdev1); >>>>>>>> if (err) { >>>>>>>> dev_err(data->dev, "Can't enable cdev1: %d\n", err); >>>>>>>> @@ -109,6 +118,7 @@ int tegra_asoc_utils_set_ac97_rate(struct >>>>>>>> tegra_asoc_utils_data *data) >>>>>>>> int err; >>>>>>>> clk_disable_unprepare(data->clk_cdev1); >>>>>>>> + clk_disable_unprepare(data->clk_extern1); >>>>>>>> clk_disable_unprepare(data->clk_pll_a_out0); >>>>>>>> clk_disable_unprepare(data->clk_pll_a); >>>>>>>> @@ -142,6 +152,14 @@ int tegra_asoc_utils_set_ac97_rate(struct >>>>>>>> tegra_asoc_utils_data *data) >>>>>>>> return err; >>>>>>>> } >>>>>>>> + if (!IS_ERR_OR_NULL(data->clk_extern1)) { >>>>>>>> + err = clk_prepare_enable(data->clk_extern1); >>>>>>>> + if (err) { >>>>>>>> + dev_err(data->dev, "Can't enable extern1: %d\n", err); >>>>>>>> + return err; >>>>>>>> + } >>>>>>>> + } >>>>>>> Why this is needed given that clk_extern1 is either a child of >>>>>>> MCLK or >>>>>>> MCLK itself (on T20)? The child clocks are enabled when the >>>>>>> parent is >>>>>>> enabled. >>>>>> For T30 and later, clk_extern1 is one of the source for >>>>>> clk_out_1_mux. >>>>>> clk_extern1 is in CAR and it has its own gate and mux. >>>>>> >>>>>> As audio mclk related clocks (clk_out_1, clk_out_1_mux, and extern1) >>>>>> are moved into ASoC driver from clock driver >>>>>> >>>>>> need to enable extern1 gate as well along with clk_out1 for T30 >>>>>> through T210. >>>>>> >>>>>> Just FYI, extern1 enable here happens only when data->clk_extern1 is >>>>>> available which is for T30 onwards. >>>>>> >>>>>>>> err = clk_prepare_enable(data->clk_cdev1); >>>>>>>> if (err) { >>>>>>>> dev_err(data->dev, "Can't enable cdev1: %d\n", err); >>>>>>>> @@ -158,6 +176,7 @@ >>>>>>>> EXPORT_SYMBOL_GPL(tegra_asoc_utils_set_ac97_rate); >>>>>>>> int tegra_asoc_utils_init(struct tegra_asoc_utils_data *data, >>>>>>>> struct device *dev) >>>>>>>> { >>>>>>>> + struct clk *clk_out_1_mux; >>>>>>>> int ret; >>>>>>>> data->dev = dev; >>>>>>>> @@ -196,6 +215,51 @@ int tegra_asoc_utils_init(struct >>>>>>>> tegra_asoc_utils_data *data, >>>>>>>> goto err_put_pll_a_out0; >>>>>>>> } >>>>>>> In a previous patch you added fallback to EXTPERIPH when >>>>>>> clk_get(MCLK) >>>>>>> fails. This will work perfectly fine for the older kernels which >>>>>>> have >>>>>>> all clocks in the same single CaR driver, but this may not work that >>>>>>> great for the newer kernels because PMC driver isn't registered >>>>>>> early >>>>>>> during boot and thus it is possible to get a legit -EPROBE_DEFER >>>>>>> which >>>>>>> shouldn't be ignored. In other words, you need to add into this >>>>>>> patch a >>>>>>> check for the error code returned by clk_get(MCLK) and fallback >>>>>>> only for >>>>>>> -EINVAL. >>>>>> yeah right, will add check in next version. >>>>>>>> + /* >>>>>>>> + * If clock parents are not set in DT, configure here to use >>>>>>>> clk_out_1 >>>>>>>> + * as mclk and extern1 as parent for Tegra30 and higher. >>>>>>>> + */ >>>>>>>> + if (!of_find_property(dev->of_node, "assigned-clock-parents", >>>>>>>> NULL) && >>>>>>>> + data->soc > TEGRA_ASOC_UTILS_SOC_TEGRA20) { >>>>>>>> + data->clk_extern1 = clk_get_sys("clk_out_1", "extern1"); >>>>>>>> + if (IS_ERR(data->clk_extern1)) { >>>>>>>> + dev_err(data->dev, "Can't retrieve clk extern1\n"); >>>>>>>> + ret = PTR_ERR(data->clk_extern1); >>>>>>>> + goto err_put_cdev1; >>>>>>>> + } >>>>>>>> + >>>>>>>> + ret = clk_set_parent(data->clk_extern1, >>>>>>>> data->clk_pll_a_out0); >>>>>>>> + if (ret < 0) { >>>>>>>> + dev_err(data->dev, >>>>>>>> + "Set parent failed for clk extern1: %d\n", >>>>>>>> + ret); >>>>>>>> + goto err_put_cdev1; >>>>>>>> + } >>>>>>>> + >>>>>>>> + clk_out_1_mux = clk_get_sys(NULL, "clk_out_1_mux"); >>>>>>> Note1: clk_get(dev, "clk_out_1_mux") should work here by letting clk >>>>>>> core to fall back to the clk_get_sys() by itself. Either way should >>>>>>> be good. >>>>> clk_get uses device rather and dev_id will be name of this device and >>>>> when clk_get fall back to __clk_get_sys() it still will use dev id of >>>>> this device rather than actual dev_id that pmc clocks are added to the >>>>> lookup. So clk_get_sys() seems to be correct to use as we can specify >>>>> exact dev_id and con_id. >>>> It should be better to use something "resource managed", thus >>>> devm_clk_get() should be a better choice. >>>> >>>>> Also, clk_find retrieves clock from lookup only when it finds matching >>>>> clock with both dev_id and con_id as pmc clocks are registered with >>>>> both >>>>> dev_id and con_id. >>>>> >>>>> I see existing clock driver adds both extern and pmc clocks >>>>> (clk_out) to >>>>> lookup with same dev_id of clk_out_1/2/3 and con_id of extern1/2/3 and >>>>> with this always extern clock will be retrieved and this is probably >>>>> because old DT and audio driver always uses extern1 rather than actual >>>>> clk_out_1 >>>>> >>>>> But this need to be fixed now as we changed to use clk_out directly >>>>> rather than extern (even for other pmc clocks) to match actual hw >>>>> design. >>>>> >>>>> Will fix this as well to register pmc clocks using con_id as >>>>> clk_out_1/2/3 in pmc driver and extern clocks using con_id of >>>>> extern1/2/3 with dev_id being NULL so we can retrieve these clocks by >>>>> just using con_id only using clk_get_sys as we switched to use >>>>> clk_out_1 >>>>> directly as pmc clock rather than extern from DT and no longer need to >>>>> pair pmc clocks to extern clocks. >>>> I'm not sure it's worth the effort to care about con_ids if implicit >>>> fallback to clk_get_sys(NULL, "...") does the right thing for the audio >>>> driver. >>>> >>>> IIRC, CCF uses variant of matching clocks by names, although I'm not >>>> sure whether that applies to older stable kernels. >>>> >>>> [snip] >>> Current clock driver adds EXTERN clock to lookup with dev_id as >>> clk_out_1/2/3 and con_id as extern_1/2/3 >>> >>> With this we can retrieve clock from lookup only with clk_get_sys where >>> we can pass dev_id as clk_out_1/2/3 and con_id as extern_1/2/3. >>> >>> We cant use devm_clk_get() or clk_get() for retrieving clocks from >>> lookup by passing device object from sound driver as dev_id will be diff >>> and clk_find will return NULL. >> Have you actually tried to test that it fails? I think it's a false >> assumption. > > Yes I tried and looking at devm_clk_get it falls back to __clk_get_sys > which uses dev_id as dev_name of the device we pass from audio driver. > > looking into clk_find API implementation it doesn't find the clock from > lookup unless both dev_id and con_id matches if a clock is added to > lookup with both dev_id and clk_id > > >>> But with the fix of having dev_id as NULL and use only con_id to add to >>> lookup, we can use resource managed APIs devm_clk_get. >>> >>> So was saying will fix this in clock driver as part of removing >>> clk_out_1/2/3 ids and pmc init from clock driver so we can use >>> devm_clk_get API in audio driver. >>> Ok
diff --git a/sound/soc/tegra/tegra_asoc_utils.c b/sound/soc/tegra/tegra_asoc_utils.c index 536a578e9512..8e3a3740df7c 100644 --- a/sound/soc/tegra/tegra_asoc_utils.c +++ b/sound/soc/tegra/tegra_asoc_utils.c @@ -60,6 +60,7 @@ int tegra_asoc_utils_set_rate(struct tegra_asoc_utils_data *data, int srate, data->set_mclk = 0; clk_disable_unprepare(data->clk_cdev1); + clk_disable_unprepare(data->clk_extern1); clk_disable_unprepare(data->clk_pll_a_out0); clk_disable_unprepare(data->clk_pll_a); @@ -89,6 +90,14 @@ int tegra_asoc_utils_set_rate(struct tegra_asoc_utils_data *data, int srate, return err; } + if (!IS_ERR_OR_NULL(data->clk_extern1)) { + err = clk_prepare_enable(data->clk_extern1); + if (err) { + dev_err(data->dev, "Can't enable extern1: %d\n", err); + return err; + } + } + err = clk_prepare_enable(data->clk_cdev1); if (err) { dev_err(data->dev, "Can't enable cdev1: %d\n", err); @@ -109,6 +118,7 @@ int tegra_asoc_utils_set_ac97_rate(struct tegra_asoc_utils_data *data) int err; clk_disable_unprepare(data->clk_cdev1); + clk_disable_unprepare(data->clk_extern1); clk_disable_unprepare(data->clk_pll_a_out0); clk_disable_unprepare(data->clk_pll_a); @@ -142,6 +152,14 @@ int tegra_asoc_utils_set_ac97_rate(struct tegra_asoc_utils_data *data) return err; } + if (!IS_ERR_OR_NULL(data->clk_extern1)) { + err = clk_prepare_enable(data->clk_extern1); + if (err) { + dev_err(data->dev, "Can't enable extern1: %d\n", err); + return err; + } + } + err = clk_prepare_enable(data->clk_cdev1); if (err) { dev_err(data->dev, "Can't enable cdev1: %d\n", err); @@ -158,6 +176,7 @@ EXPORT_SYMBOL_GPL(tegra_asoc_utils_set_ac97_rate); int tegra_asoc_utils_init(struct tegra_asoc_utils_data *data, struct device *dev) { + struct clk *clk_out_1_mux; int ret; data->dev = dev; @@ -196,6 +215,51 @@ int tegra_asoc_utils_init(struct tegra_asoc_utils_data *data, goto err_put_pll_a_out0; } + /* + * If clock parents are not set in DT, configure here to use clk_out_1 + * as mclk and extern1 as parent for Tegra30 and higher. + */ + if (!of_find_property(dev->of_node, "assigned-clock-parents", NULL) && + data->soc > TEGRA_ASOC_UTILS_SOC_TEGRA20) { + data->clk_extern1 = clk_get_sys("clk_out_1", "extern1"); + if (IS_ERR(data->clk_extern1)) { + dev_err(data->dev, "Can't retrieve clk extern1\n"); + ret = PTR_ERR(data->clk_extern1); + goto err_put_cdev1; + } + + ret = clk_set_parent(data->clk_extern1, data->clk_pll_a_out0); + if (ret < 0) { + dev_err(data->dev, + "Set parent failed for clk extern1: %d\n", + ret); + goto err_put_cdev1; + } + + clk_out_1_mux = clk_get_sys(NULL, "clk_out_1_mux"); + if (IS_ERR(clk_out_1_mux)) { + dev_err(data->dev, "Can't retrieve clk clk_out_1_mux\n"); + ret = PTR_ERR(clk_out_1_mux); + goto err_put_cdev1; + } + + ret = clk_set_parent(clk_out_1_mux, data->clk_extern1); + if (ret < 0) { + dev_err(data->dev, + "Set parent failed for clk_out_1_mux: %d\n", + ret); + clk_put(clk_out_1_mux); + goto err_put_cdev1; + } + + data->clk_cdev1 = clk_get_sys(NULL, "clk_out_1"); + if (IS_ERR(data->clk_cdev1)) { + dev_err(data->dev, "Can't retrieve clk clk_out_1\n"); + ret = PTR_ERR(data->clk_cdev1); + goto err_put_cdev1; + } + } + ret = tegra_asoc_utils_set_rate(data, 44100, 256 * 44100); if (ret) goto err_put_cdev1; @@ -215,6 +279,8 @@ EXPORT_SYMBOL_GPL(tegra_asoc_utils_init); void tegra_asoc_utils_fini(struct tegra_asoc_utils_data *data) { + if (!IS_ERR_OR_NULL(data->clk_extern1)) + clk_put(data->clk_extern1); clk_put(data->clk_cdev1); clk_put(data->clk_pll_a_out0); clk_put(data->clk_pll_a); diff --git a/sound/soc/tegra/tegra_asoc_utils.h b/sound/soc/tegra/tegra_asoc_utils.h index 0c13818dee75..5f2b96478caf 100644 --- a/sound/soc/tegra/tegra_asoc_utils.h +++ b/sound/soc/tegra/tegra_asoc_utils.h @@ -25,6 +25,7 @@ struct tegra_asoc_utils_data { struct clk *clk_pll_a; struct clk *clk_pll_a_out0; struct clk *clk_cdev1; + struct clk *clk_extern1; int set_baseclock; int set_mclk; };
Current ASoC driver uses extern1 as cdev1 clock from Tegra30 onwards through device tree. Actual audio mclk is clk_out_1 and to use PLLA for mclk rate control, need to clk_out_1_mux parent to extern1 and extern1 parent to PLLA_OUT0. Currently Tegra clock driver init sets the parents and enables both clk_out_1 and extern1 clocks. But these clocks parent and enables should be controlled by ASoC driver. Clock parents can be specified in device tree using assigned-clocks and assigned-clock-parents. To enable audio mclk, both clk_out_1 and extern1 clocks need to be enabled. This patch configures parents for clk_out_1 and extern1 clocks if device tree does not specify clock parents inorder to support old device tree and controls mclk using both clk_out_1 and extern1 clocks. Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com> --- sound/soc/tegra/tegra_asoc_utils.c | 66 ++++++++++++++++++++++++++++++++++++++ sound/soc/tegra/tegra_asoc_utils.h | 1 + 2 files changed, 67 insertions(+)