From: "Yang, Wenyou" <Wenyou.Yang@atmel.com> To: Mark Rutland <mark.rutland@arm.com> Cc: "Ferre, Nicolas" <Nicolas.FERRE@atmel.com>, "linux@arm.linux.org.uk" <linux@arm.linux.org.uk>, "sylvain.rochet@finsecur.com" <sylvain.rochet@finsecur.com>, "Vilchez, Patrice" <Patrice.VILCHEZ@atmel.com>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "alexandre.belloni@free-electrons.com" <alexandre.belloni@free-electrons.com>, "peda@axentia.se" <peda@axentia.se>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org> Subject: RE: [PATCH 6/7] pm: at91: add achieve the mpddrc peripheral ID and the DDR clock ID support Date: Tue, 27 Jan 2015 05:24:51 +0000 [thread overview] Message-ID: <B256D81BAE5131468A838E5D7A243641BFD35D20@penmbx01> (raw) In-Reply-To: <20150126114956.GE23313@leverpostej> Hi Mark, Thank you for your review. > -----Original Message----- > From: Mark Rutland [mailto:mark.rutland@arm.com] > Sent: Monday, January 26, 2015 7:50 PM > To: Yang, Wenyou > Cc: Ferre, Nicolas; linux@arm.linux.org.uk; sylvain.rochet@finsecur.com; Vilchez, > Patrice; linux-kernel@vger.kernel.org; alexandre.belloni@free-electrons.com; > peda@axentia.se; linux-arm-kernel@lists.infradead.org > Subject: Re: [PATCH 6/7] pm: at91: add achieve the mpddrc peripheral ID and the > DDR clock ID support > > On Mon, Jan 26, 2015 at 10:08:16AM +0000, Wenyou Yang wrote: > > The patch achieves the mpddr controller peripheral ID and the DDR > > clock ID from the dts file. > > > > They will be used in the future to disable the mpddr controller'c > > clock the and DDR clock to decrease the power consumption during suspending. > > > > Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com> > > --- > > arch/arm/mach-at91/generic.h | 2 ++ > > arch/arm/mach-at91/setup.c | 24 ++++++++++++++++++++++++ > > 2 files changed, 26 insertions(+) > > > > diff --git a/arch/arm/mach-at91/generic.h > > b/arch/arm/mach-at91/generic.h index 41796bf..3c72a3e 100644 > > --- a/arch/arm/mach-at91/generic.h > > +++ b/arch/arm/mach-at91/generic.h > > @@ -47,6 +47,8 @@ void __init at91_sam9x5_pm_init(void) { } struct > > at91_pm_struct { > > unsigned long uhp_udp_mask; > > int memctrl; > > + u32 mpddrc_id[2]; > > + u32 ddrck_id; > > }; > > > > #endif /* _AT91_GENERIC_H */ > > diff --git a/arch/arm/mach-at91/setup.c b/arch/arm/mach-at91/setup.c > > index 7924663..a306f95 100644 > > --- a/arch/arm/mach-at91/setup.c > > +++ b/arch/arm/mach-at91/setup.c > > @@ -363,6 +363,27 @@ void __init at91_ioremap_matrix(u32 base_addr) > > panic(pr_fmt("Impossible to ioremap at91_matrix_base\n")); } > > > > +static u32 at91_of_get_ddr_id(struct device_node *np, char *name) { > > + struct of_phandle_args clkspec; > > + u32 id; > > + int index; > > + int rc; > > + > > + index = of_property_match_string(np, "clock-names", name); > > + rc = of_parse_phandle_with_args(np, "clocks", "#clock-cells", index, > &clkspec); > > + if (rc) > > + return 0; > > + > > + rc = of_property_read_u32(clkspec.np, "reg", &id); > > + if (rc) > > + return 0; > > + > > + of_node_put(clkspec.np); > > + > > + return id; > > +} > > This doesn't look right to me. This assumes the format of the clock provider node, > which invalidates the point of having the abstraction in the first place. > > > + > > struct at91_ramc_of_data { > > u8 ramc_type; > > }; > > @@ -400,6 +421,9 @@ static void at91_dt_ramc(void) > > of_data = of_id->data; > > at91_pm_data.memctrl = of_data->ramc_type; > > > > + at91_pm_data.mpddrc_id[idx] = at91_of_get_ddr_id(np, "mpddr"); > > + at91_pm_data.ddrck_id = at91_of_get_ddr_id(np, "ddrck"); > > + > > Why do you need these here? > > Surely the logic for poking any clocks should live in the relevant clock controller > drivers? Thank for your suggestion. I thought that it is reasonable to get the DDR controller's peripheral id from the DDR device node. Anyway, let me think over how to do it. > > Mark. Best Regards, Wenyou Yang
WARNING: multiple messages have this Message-ID (diff)
From: Wenyou.Yang@atmel.com (Yang, Wenyou) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 6/7] pm: at91: add achieve the mpddrc peripheral ID and the DDR clock ID support Date: Tue, 27 Jan 2015 05:24:51 +0000 [thread overview] Message-ID: <B256D81BAE5131468A838E5D7A243641BFD35D20@penmbx01> (raw) In-Reply-To: <20150126114956.GE23313@leverpostej> Hi Mark, Thank you for your review. > -----Original Message----- > From: Mark Rutland [mailto:mark.rutland at arm.com] > Sent: Monday, January 26, 2015 7:50 PM > To: Yang, Wenyou > Cc: Ferre, Nicolas; linux at arm.linux.org.uk; sylvain.rochet at finsecur.com; Vilchez, > Patrice; linux-kernel at vger.kernel.org; alexandre.belloni at free-electrons.com; > peda at axentia.se; linux-arm-kernel at lists.infradead.org > Subject: Re: [PATCH 6/7] pm: at91: add achieve the mpddrc peripheral ID and the > DDR clock ID support > > On Mon, Jan 26, 2015 at 10:08:16AM +0000, Wenyou Yang wrote: > > The patch achieves the mpddr controller peripheral ID and the DDR > > clock ID from the dts file. > > > > They will be used in the future to disable the mpddr controller'c > > clock the and DDR clock to decrease the power consumption during suspending. > > > > Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com> > > --- > > arch/arm/mach-at91/generic.h | 2 ++ > > arch/arm/mach-at91/setup.c | 24 ++++++++++++++++++++++++ > > 2 files changed, 26 insertions(+) > > > > diff --git a/arch/arm/mach-at91/generic.h > > b/arch/arm/mach-at91/generic.h index 41796bf..3c72a3e 100644 > > --- a/arch/arm/mach-at91/generic.h > > +++ b/arch/arm/mach-at91/generic.h > > @@ -47,6 +47,8 @@ void __init at91_sam9x5_pm_init(void) { } struct > > at91_pm_struct { > > unsigned long uhp_udp_mask; > > int memctrl; > > + u32 mpddrc_id[2]; > > + u32 ddrck_id; > > }; > > > > #endif /* _AT91_GENERIC_H */ > > diff --git a/arch/arm/mach-at91/setup.c b/arch/arm/mach-at91/setup.c > > index 7924663..a306f95 100644 > > --- a/arch/arm/mach-at91/setup.c > > +++ b/arch/arm/mach-at91/setup.c > > @@ -363,6 +363,27 @@ void __init at91_ioremap_matrix(u32 base_addr) > > panic(pr_fmt("Impossible to ioremap at91_matrix_base\n")); } > > > > +static u32 at91_of_get_ddr_id(struct device_node *np, char *name) { > > + struct of_phandle_args clkspec; > > + u32 id; > > + int index; > > + int rc; > > + > > + index = of_property_match_string(np, "clock-names", name); > > + rc = of_parse_phandle_with_args(np, "clocks", "#clock-cells", index, > &clkspec); > > + if (rc) > > + return 0; > > + > > + rc = of_property_read_u32(clkspec.np, "reg", &id); > > + if (rc) > > + return 0; > > + > > + of_node_put(clkspec.np); > > + > > + return id; > > +} > > This doesn't look right to me. This assumes the format of the clock provider node, > which invalidates the point of having the abstraction in the first place. > > > + > > struct at91_ramc_of_data { > > u8 ramc_type; > > }; > > @@ -400,6 +421,9 @@ static void at91_dt_ramc(void) > > of_data = of_id->data; > > at91_pm_data.memctrl = of_data->ramc_type; > > > > + at91_pm_data.mpddrc_id[idx] = at91_of_get_ddr_id(np, "mpddr"); > > + at91_pm_data.ddrck_id = at91_of_get_ddr_id(np, "ddrck"); > > + > > Why do you need these here? > > Surely the logic for poking any clocks should live in the relevant clock controller > drivers? Thank for your suggestion. I thought that it is reasonable to get the DDR controller's peripheral id from the DDR device node. Anyway, let me think over how to do it. > > Mark. Best Regards, Wenyou Yang
next prev parent reply other threads:[~2015-01-27 5:27 UTC|newest] Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-01-26 10:03 [PATCH 0/7] AT91 pm improvements for 3.20 Wenyou Yang 2015-01-26 10:03 ` Wenyou Yang 2015-01-26 10:04 ` [PATCH 1/7] pm: at91: achieve the memory controller's type from the dts file Wenyou Yang 2015-01-26 10:04 ` Wenyou Yang 2015-01-26 10:06 ` [PATCH 2/7] pm: at91: pm_suspend: add the WFI support for ARMv7 Wenyou Yang 2015-01-26 10:06 ` Wenyou Yang 2015-01-26 13:05 ` Sergei Shtylyov 2015-01-26 13:05 ` Sergei Shtylyov 2015-01-27 4:44 ` Yang, Wenyou 2015-01-27 4:44 ` Yang, Wenyou 2015-01-28 11:25 ` Lorenzo Pieralisi 2015-01-28 11:25 ` Lorenzo Pieralisi 2015-01-29 2:36 ` Yang, Wenyou 2015-01-29 2:36 ` Yang, Wenyou 2015-01-29 12:22 ` Lorenzo Pieralisi 2015-01-29 12:22 ` Lorenzo Pieralisi 2015-01-30 7:23 ` Yang, Wenyou 2015-01-30 7:23 ` Yang, Wenyou 2015-01-30 10:17 ` Lorenzo Pieralisi 2015-01-30 10:17 ` Lorenzo Pieralisi 2015-01-30 10:44 ` Lorenzo Pieralisi 2015-01-30 10:44 ` Lorenzo Pieralisi 2015-01-26 10:06 ` [PATCH 3/7] pm: at91: pm_suspend: MOR register KEY was missing Wenyou Yang 2015-01-26 10:06 ` Wenyou Yang 2015-01-26 10:07 ` [PATCH 4/7] ARM: at91: enable the L2 Cache controller Wenyou Yang 2015-01-26 10:07 ` Wenyou Yang 2015-01-26 11:46 ` Mark Rutland 2015-01-26 11:46 ` Mark Rutland 2015-01-26 12:45 ` Russell King - ARM Linux 2015-01-26 12:45 ` Russell King - ARM Linux 2015-01-26 22:36 ` Alexandre Belloni 2015-01-26 22:36 ` Alexandre Belloni 2015-01-27 5:11 ` Yang, Wenyou 2015-01-27 5:11 ` Yang, Wenyou 2015-01-26 10:07 ` [PATCH 5/7] pm: at91: add disable/enable the L1/L2 cache while suspend/resume Wenyou Yang 2015-01-26 10:07 ` Wenyou Yang 2015-01-26 10:08 ` [PATCH 6/7] pm: at91: add achieve the mpddrc peripheral ID and the DDR clock ID support Wenyou Yang 2015-01-26 10:08 ` Wenyou Yang 2015-01-26 11:49 ` Mark Rutland 2015-01-26 11:49 ` Mark Rutland 2015-01-27 5:24 ` Yang, Wenyou [this message] 2015-01-27 5:24 ` Yang, Wenyou 2015-01-26 10:08 ` [PATCH 7/7] pm: at91: add disable/enable the mpddrc's clock and DDR clock support Wenyou Yang 2015-01-26 10:08 ` Wenyou Yang
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=B256D81BAE5131468A838E5D7A243641BFD35D20@penmbx01 \ --to=wenyou.yang@atmel.com \ --cc=Nicolas.FERRE@atmel.com \ --cc=Patrice.VILCHEZ@atmel.com \ --cc=alexandre.belloni@free-electrons.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux@arm.linux.org.uk \ --cc=mark.rutland@arm.com \ --cc=peda@axentia.se \ --cc=sylvain.rochet@finsecur.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.