On Mon, Aug 22, 2016 at 07:22:47PM +0200, Sylwester Nawrocki wrote: > On 08/22/2016 11:22 AM, Charles Keepax wrote: > > Yeah I am not sure this is quite the correct approach, there are > > quite a few corner cases that would not be covered well here. For > > example an internally divided down 32k in which case both the 32k > > and MCLK would come from the same pin, or using the 32k to feed > > an FLL in which case we are trying to enable MCLK1 unnecessarily. > > > > I think we could request the 32k clock source from this part > > of the code, but without implementing clock drivers for the > > chips internal clocking I think the main MCLK would need to be > > requested from the machine driver for now. > > > > On that note, I have been working on a patch chain that adds an > > actual clock driver for the chip unfortunately this has been > > delayed somewhat due to issues interfacing SPI backed clocks to > > the clock framework. Krzysztof Kozlowski has sent a series that > > appears to resolve these issues for me so hopefully once the > > clock guys have had a look at that I can send my clock driver. > > My current implementation mostly just implements the 32k clock > > but we can start building that out to include the SYSCLKs and > > FLLs etc. fairly quickly. The best thing might be to wait for > > that and then build additional features onto that. > > > > Let me know if you want to get an early look at that code. > > Thanks for the feedback. I would really like to avoid touching > this code and messing up anything in code shared by multiple > CODECs. You certainly know it much better than than I do. > > I got suggestion from Mark not to request the main MCLK clock > in the machine driver. But even if gating of that clock was > added to the CODEC driver I would need to get hold of it in > the machine driver to get rate of MCLK. So I thought about > exporting a helper from the MFD for requesting MCLK clock or > specifying MCLK clock back in the sound DT node. > > IIUC, you are suggesting to leave the 32k parts of the patch > and just drop the MCLK part? > I would certainly be ok with that it is very similar to what my patch does but done from the MFD driver rather than moving things out into a clock driver. Although it looks like in this case we need to solve both clocks for it to be useful to you. > If we provided an interface like: > > struct clk * arizona_get_mclk(struct arizona *arizona, int id); > void arizona_put_mclk(struct clk *clk); > > the machine driver would need to get hold of struct arizona*, > which is not that straightforward if we are given on CODEC > of_node (it can be a child of I2S or SPI bus). > > Then how about specifying MCLK in the sound node and using > regular devm_clk_get()? > Yeah I think we want to avoid specifying the MCLK in the sound node as it really is a clock the codec consumes so should be defined there in the DT. > Regarding the clock locking patches, I think it needs some more > discussion and should rather be merged early in the development > cycle to have good exposure in -next as it's quite an invasive > change. > I would agree there, and I am not sure how clear it is what the clock guys will make of the approach yet as well. Which might cause issues if we want to merge something now. > I'd be happy to look at your code, if only to have a better > overview and to avoid interfering with you work. > I am afraid I haven't managed to get time today but I will fire you an email with my current work in progress stuff, hopefully tomorrow. > Anyway, my main goal is only to get the tm2_wm5110 sound card > upstream, with as little casualties as possible;) Apologies it seems I missed another version of this on the mailing list, please do feel free to add me or the patches@xxxxxxxxxxxxxxxxxxxxxxxxxxx list as a direct CC on any patches using our CODECs, I am always more than happy to look over things and feel bad when I miss stuff. I will have a look at the latest version of the patch. I think we should be able to do something requesting the 32k approximately as your existing patch here does, but then requesting the main MCLK from the set_pll and set_sysclk handlers. Eventually I would like the internal SYSCLK and FLLs represented in the clock framework, so I want to have a quick think about how that would migrate over. Let me see what I can come up with here. Thanks, Charles _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
