11.01.2020 02:14, Sowjanya Komatineni пишет: > > On 1/10/20 3:02 PM, Dmitry Osipenko wrote: >> External email: Use caution opening links or attachments >> >> >> 11.01.2020 01:13, Sowjanya Komatineni пишет: >>> On 1/10/20 2:05 PM, Dmitry Osipenko wrote: >>>> External email: Use caution opening links or attachments >>>> >>>> >>>> 10.01.2020 20:04, Sowjanya Komatineni пишет: >>>>> On 1/9/20 10:52 AM, Sowjanya Komatineni wrote: >>>>>> On 1/7/20 10:28 PM, Sameer Pujar wrote: >>>>>>> On 1/8/2020 11:18 AM, Sowjanya Komatineni wrote: >>>>>>>> On 1/7/20 9:34 PM, Sameer Pujar wrote: >>>>>>>>> On 1/8/2020 9:55 AM, Sowjanya Komatineni wrote: >>>>>>>>>> mclk is from clk_out_1 which is part of Tegra PMC block and pmc >>>>>>>>>> clocks >>>>>>>>>> are moved to Tegra PMC driver with pmc as clock provider and >>>>>>>>>> using >>>>>>>>>> pmc >>>>>>>>>> clock ids. >>>>>>>>>> >>>>>>>>>> New device tree uses clk_out_1 from pmc clock provider. >>>>>>>>>> >>>>>>>>>> So, this patch adds implementation for mclk fallback to extern1 >>>>>>>>>> when >>>>>>>>>> retrieving mclk returns -ENOENT to be backward compatible of new >>>>>>>>>> device >>>>>>>>>> tree with older kernels. >>>>>>>>>> >>>>>>>>>> Tested-by: Dmitry Osipenko <digetx@xxxxxxxxx> >>>>>>>>>> Reviewed-by: Dmitry Osipenko <digetx@xxxxxxxxx> >>>>>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@xxxxxxxxxx> >>>>>>>>>> --- >>>>>>>>>> sound/soc/tegra/tegra_asoc_utils.c | 11 ++++++++++- >>>>>>>>>> 1 file changed, 10 insertions(+), 1 deletion(-) >>>>>>>>>> >>>>>>>>>> diff --git a/sound/soc/tegra/tegra_asoc_utils.c >>>>>>>>>> b/sound/soc/tegra/tegra_asoc_utils.c >>>>>>>>>> index 9cfebef74870..9a5f81039491 100644 >>>>>>>>>> --- a/sound/soc/tegra/tegra_asoc_utils.c >>>>>>>>>> +++ b/sound/soc/tegra/tegra_asoc_utils.c >>>>>>>>>> @@ -183,7 +183,16 @@ int tegra_asoc_utils_init(struct >>>>>>>>>> tegra_asoc_utils_data *data, >>>>>>>>>> data->clk_cdev1 = devm_clk_get(dev, "mclk"); >>>>>>>>>> if (IS_ERR(data->clk_cdev1)) { >>>>>>>>>> dev_err(data->dev, "Can't retrieve clk cdev1\n"); >>>>>>>>> This error print can be moved inside below if, when this actually >>>>>>>>> meant to be an error condition. >>>>>>>>> >>>>>>>> Want to show error even if mclk retrieval returns ENOENT to clearly >>>>>>>> indicate mclk does not exist along with message of falling back to >>>>>>>> extern1. >>>>>>> Yes, but falling back essentially means 'mclk' is not available and >>>>>>> fallback print is not an error. >>>>>>> Not a major issue though, you can consider updating. Otherwise LGTM. >>>>>>> >>>>>> Will update >>>>>>>>>> - return PTR_ERR(data->clk_cdev1); >>>>>>>>>> + if (PTR_ERR(data->clk_cdev1) != -ENOENT) >>>>>>>>>> + return PTR_ERR(data->clk_cdev1); >>>>>>>>>> + /* Fall back to extern1 */ >>>>>>>>>> + data->clk_cdev1 = devm_clk_get(dev, "extern1"); >>>>>>>>>> + if (IS_ERR(data->clk_cdev1)) { >>>>>>>>>> + dev_err(data->dev, "Can't retrieve clk extern1\n"); >>>>>>>>>> + return PTR_ERR(data->clk_cdev1); >>>>>>>>>> + } >>>>>>>>>> + >>>>>>>>>> + dev_err(data->dev, "Falling back to extern1\n"); >>>>>>>>> This can be a info print? >>>>>> Will use dev_info >>>>>>>>>> } >>>>>>>>>> /* >>>>>> Dmitry/Rob, there was discussion in v3 regarding backporting mclk >>>>>> fallback. >>>>>> >>>>>> Dmitry wanted Rob to confirm on this >>>>>> >>>>>> I think openSUSE Leap could be one of those distros that use LTS >>>>>> kernel >>>>>> with newer device-trees, but that's not 100%. Maybe Rob could help >>>>>> clarifying that. >>>>>> >>>>>> Dmitry/Rob, Can you please confirm if mclk fallback patch need >>>>>> backport to have new device tree work with old kernels? >>>>>> >>>>> Dmitry, >>>>> >>>>> Can you please confirm if we need to backport this mclk fallback >>>>> patch? >>>>> >>>> Seems only T210 was making use of the CaR's TEGRA*_CLK_CLK_OUT_*, thus >>>> the backporting isn't needed. >>> Thanks Dmitry >>>> Also, please use 'git rebase --exec make ...' to make sure that all >>>> patches are compiling without problems. The removal of the legacy clock >>>> IDs should be done after the device-trees changes, otherwise it looks >>>> like DTBs compilation will fail. It's possible that the order of the >>>> patches could be changed if Thierry will chose to split this series >>>> into >>>> several pull requests, nevertheless all patches should compile and work >>>> in the original order. >>> OK, Will move patches of device tree updates to use new DT ID prior to >>> removal of old ID. >> Oh, wait. But then the newer device-trees won't work with the stable >> kernels, so it actually won't hurt to backport this change. > ok will add Fixes tag to have this mclk fallback patch backported. >> >> Secondly, please move the "Use device managed resource APIs to get the >> clock" after the ASoC patches with the stable tags, such that the stable >> patches could be applied cleanly. > OK >> >> Lastly, please separate the assigned-clocks change from the the audio >> mclk enable/disable into a standalone patch. These changes are not >> interdependent, unless I'm missing something. > > But parent configuration when assigned-clock-parents are not in DT are > needed along with mclk enable > > as we are removing audio clocks parent configuration and enabling them > together from clock driver. > > So doesn't both parent configuration and enabling mclk together need to > be in same patch to match what we are removing from clock driver? > All current stable kernels happen to work without any visible problems because of the non-critical clk-enable refcounting bug that masks the problem. Thus the mclk will be enabled in stable kernels without any extra changes and the assigned-clock-parents shouldn't affect that. Please make sure that every patch in this series: 1) Compiles without any errors and warnings. 2) Works, i.e. you should be able to checkout any commit and kernel should boot/work without any regressions. 3) Stable patches could be cherry-picked into stable kernels without merge conflicts. To achieve that you'll need to sort patches in the correct order and do some basic testing.