From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Date: Tue, 07 Jan 2014 09:34:38 +0000 Subject: Re: [PATCH] ARM: shmobile: r8a7790: add audio clocks Message-Id: <2165619.cOvkiAUANQ@avalon> List-Id: References: <8738l1jzfv.wl%kuninori.morimoto.gx@gmail.com> In-Reply-To: <8738l1jzfv.wl%kuninori.morimoto.gx@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: linux-sh@vger.kernel.org Hi Morimoto-san, Thank you for the patch. On Monday 06 January 2014 00:18:31 Kuninori Morimoto wrote: > From: Kuninori Morimoto > > Signed-off-by: Kuninori Morimoto > --- > > >> Laurent > > Could you please check this patch ? > > >> Simon > > This patch is based on renesas-devel-v3.13-rc5-20131230 tag > > arch/arm/boot/dts/r8a7790.dtsi | 29 +++++++++++++++++++++++++++++ > arch/arm/mach-shmobile/clock-r8a7790.c | 16 ++++++++++++++++ > 2 files changed, 45 insertions(+) > > diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi > index f48487c..abe68b9 100644 > --- a/arch/arm/boot/dts/r8a7790.dtsi > +++ b/arch/arm/boot/dts/r8a7790.dtsi > @@ -312,6 +312,27 @@ > clock-frequency = <0>; > clock-output-names = "extal"; > }; > + audio_clk_a: audio_clk_a { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + /* This value can be overriden by the board if exist */ > + clock-frequency = <0>; > + clock-output-names = "audio_clk_a"; > + }; > + audio_clk_b: audio_clk_b { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + /* This value can be overriden by the board if exist */ > + clock-frequency = <0>; > + clock-output-names = "audio_clk_b"; > + }; > + audio_clk_c: audio_clk_c { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + /* This value can be overriden by the board if exist */ > + clock-frequency = <0>; > + clock-output-names = "audio_clk_c"; > + }; There three clocks could be provided by an external clock generator, so "fixed-clock" might not be the right compatible string and might need to be overriden by the board as well. I would thus replace the three comments by a single one on top of the audio_clk_* nodes that goes like "The external audio clocks are configured as 0 Hz fixed frequency clocks by default. Boards that provide audio clocks should override them." > /* Special CPG clocks */ > cpg_clocks: cpg_clocks@e6150000 { > @@ -522,6 +543,14 @@ > clock-mult = <1>; > clock-output-names = "cp"; > }; > + audio_clk_internal: audio_clk_internal { > + compatible = "fixed-factor-clock"; > + clocks = <&m2_clk>; > + #clock-cells = <0>; > + clock-div = <1>; > + clock-mult = <1>; > + clock-output-names = "audio_clk_internal"; > + }; Do you really need an internal clock, can't you reference m2_clk directly instead ? > /* Gate clocks */ > mstp0_clks: mstp0_clks@e6150130 { > diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c > b/arch/arm/mach-shmobile/clock-r8a7790.c index c5c60ec..9dd5499b4 100644 > --- a/arch/arm/mach-shmobile/clock-r8a7790.c > +++ b/arch/arm/mach-shmobile/clock-r8a7790.c > @@ -82,6 +82,15 @@ static struct clk main_clk = { > .ops = &followparent_clk_ops, > }; > > +static struct clk audio_clk_a = { > +}; > + > +static struct clk audio_clk_b = { > +}; > + > +static struct clk audio_clk_c = { > +}; > + > /* > * clock ratio of these clock will be updated > * on r8a7790_clock_init() > @@ -115,6 +124,9 @@ SH_FIXED_RATIO_CLK_SET(ddr_clk, pll3_clk, 1, 8); > SH_FIXED_RATIO_CLK_SET(mp_clk, pll1_div2_clk, 1, 15); > > static struct clk *main_clks[] = { > + &audio_clk_a, > + &audio_clk_b, > + &audio_clk_c, > &extal_clk, > &extal_div2_clk, > &main_clk, > @@ -246,6 +258,10 @@ static struct clk mstp_clks[MSTP_NR] = { > static struct clk_lookup lookups[] = { > > /* main clocks */ > + CLKDEV_CON_ID("audio_clk_a", &audio_clk_a), > + CLKDEV_CON_ID("audio_clk_b", &audio_clk_b), > + CLKDEV_CON_ID("audio_clk_c", &audio_clk_c), > + CLKDEV_CON_ID("audio_clk_internal", &m2_clk), > CLKDEV_CON_ID("extal", &extal_clk), > CLKDEV_CON_ID("extal_div2", &extal_div2_clk), > CLKDEV_CON_ID("main", &main_clk), -- Regards, Laurent Pinchart