Hi Jiada > SMSTPCR922 controls input of two clocks "S0D1ϕ" and "S0D4ϕ", Ahh, OK I could check it via Block diagram. But it is "Module stop" for these, anyway. "Module stop" and "S0D1ϕ/S0D4ϕ" are completely different clocks. > "S0D4ϕ" is input to BRGA, > "S0D1ϕ" is input to avb_counter8 It seems we need to update "clk_i" portion first for this purpose. Then, we need to consider about Gen2 vs Gen3, etc, etc... It seems we can't keep patch simple... > > 3rd, we need to create new clock/handler for > > avb_counter8 / audio_clk_div3 / avb_div8 for "internal" purpose, > > and need to create avb_adg_syn[] clock for "external" purpose for EAVB/IF. > > Your code is creating / registering adg->clkavb[], > > but it is for avb_counter8 in my understanding. > > We don't need to register it as formal clock IMO. > I agree, besides avb_counter8 there are other clocks which need to be > added as you have mentioned, > in this patch-set, I only added avb_counter8, because I want to keep > the patch-set simple, > other clocks can be added later. No, No, No. avb_counter8 is just "internal" clock, and one of parent clock for avb_div8 which is needed for avb_adg_syn[]. We need to control/register is avb_adg_syn[], not avb_counter8. Your approach on this patch is very "Quick-Hack". I have no objection for out-of-tree patches. But, "Quick-Hack approach with changing formal DT on upstream" is nightmare. > avb_counter8 is registered as formal clock is because, > there is use case that EAVB-IF may use avb_div8, and avb_counter8 is > input to avb_div8. > if keeps avb_counter8 'internal', I am not sure how EAVB-IF can set > clock rate for avb_counter8 EAVB/IF driver setups avb_adg_syn[]. Then, ADG considers/searchs avb_counter8/avb_div8/audio_clk_div3 settings internally. Don't control avb_counter8 directly from EAVB. Best regards --- Kuninori Morimoto _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
