Re: [PATCH 09/10 V3] omap3: pm: introduce 3630 opps

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Nishanth,

Few comments bellow.

On Wed, Nov 25, 2009 at 05:09:18AM +0100, ext Nishanth Menon wrote:
> Introduce the OMAP3630 OPPs including the defined OPP tuples.
> 
> Further information on OMAP3630 can be found here:
> http://focus.ti.com/general/docs/wtbu/wtbuproductcontent.tsp?templateId=6123&navigationId=12836&contentId=52606
> 
> OMAP36xx family introduces:
> VDD1 with 4 OPPs, of which OPP3 & 4 are available only on devices yet
> to be introduced in 36xx family. Meanwhile, VDD2 has 2 opps.
> 
> Device Support of OPPs->
>            |<-3630-600->| (default)
>            |<-      3630-800    ->| (device: TBD)
>            |<-      3630-1000            ->| (device: TBD)
> H/w OPP-> OPP50 OPP100       OPP-Turbo   OPP1G-SB
> VDD1      OPP1  OPP2         OPP3        OPP4
> VDD2      OPP1  OPP2         OPP2        OPP2
> 
> Note:
> a) TI h/w naming for OPPs are now standardized in terms of OPP50, 100,
>    Turbo and SB. This maps as shown above to the opp IDs (s/w term).
> b) For boards which need custom VDD1/2 OPPs, the opp table can be
>    updated by the board file on a need basis after the
>    omap3_pm_init_opp_table call. The OPPs introduced here are the
>    official OPP table at this point in time.
> 
> Cc: Benoit Cousson <b-cousson@xxxxxx>
> Cc: Kevin Hilman <khilman@xxxxxxxxxxxxxxxxxxx>
> Cc: Madhusudhan Chikkature Rajashekar <madhu.cr@xxxxxx>
> Cc: Paul Walmsley <paul@xxxxxxxxx>
> Cc: Romit Dasgupta <romit@xxxxxx>
> Cc: Sanjeev Premi <premi@xxxxxx>
> Cc: Santosh Shilimkar <santosh.shilimkar@xxxxxx>
> Cc: Sergio Alberto Aguirre Rodriguez <saaguirre@xxxxxx>
> Cc: Thara Gopinath <thara@xxxxxx>
> Cc: Vishwanath Sripathy <vishwanath.bs@xxxxxx>
> 
> Signed-off-by: Nishanth Menon <nm@xxxxxx>
> ---
>  arch/arm/mach-omap2/pm34xx.c |   49 ++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 47 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index ad21f5f..05ecf02 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -143,6 +143,41 @@ static struct omap_opp_def __initdata omap34xx_dsp_rate_table[] = {
>  	{.enabled = 0, .freq = 0, .u_volt = 0}
>  };
>  
> +static struct omap_opp_def __initdata omap36xx_mpu_rate_table[] = {
> +	/*OPP1 - OPP50*/
> +	{.enabled = true,  .freq = 300000000, .u_volt = 930000},
> +	/*OPP2 - OPP100*/
> +	{.enabled = true,  .freq = 600000000, .u_volt = 1100000},
> +	/*OPP3 - OPP-Turbo*/
> +	{.enabled = false, .freq = 800000000, .u_volt = 1260000},
> +	/*OPP4 - OPP-SB*/
> +	{.enabled = false, .freq = 1000000000, .u_volt = 1310000},
> +	/* Terminator */
> +	{.enabled = 0, .freq = 0, .u_volt = 0}
> +};
> +
> +static struct omap_opp_def __initdata omap36xx_l3_rate_table[] = {
> +	/*OPP1 - OPP50 */
> +	{.enabled = true, .freq = 100000000, .u_volt = 930000},
> +	/*OPP2 - OPP100, OPP-Turbo, OPP-SB*/
> +	{.enabled = true, .freq = 200000000, .u_volt = 1137500},
> +	/* Terminator */
> +	{.enabled = 0, .freq = 0, .u_volt = 0}
> +};
> +
> +static struct omap_opp_def __initdata omap36xx_dsp_rate_table[] = {
> +	/*OPP1 - OPP50*/
> +	{.enabled = true,  .freq = 260000000, .u_volt = 930000},
> +	/*OPP2 - OPP100*/
> +	{.enabled = true,  .freq = 520000000, .u_volt = 1100000},
> +	/*OPP3 - OPP-Turbo*/
> +	{.enabled = false, .freq = 660000000, .u_volt = 1260000},
> +	/*OPP4 - OPP-SB*/
> +	{.enabled = false, .freq = 875000000, .u_volt = 1310000},
> +	/* Terminator */
> +	{.enabled = 0, .freq = 0, .u_volt = 0}
> +};
> +

Although it is not mandatory, I'd say this way of initializing structure array is more common:

static struct omap_opp_def __initdata omap36xx_dsp_rate_table[] = {
	/*OPP1 - OPP50*/
	{
		.enabled	= true,
		.freq		= 260000000,
		.u_volt		= 930000,
	},
	/*OPP2 - OPP100*/
	{
		.enabled	= true,
		.freq		= 520000000,
		.u_volt		= 1100000,
	},
	/*OPP3 - OPP-Turbo*/
	{
		.enabled	= false,
		.freq		= 660000000,
		.u_volt		= 1260000,
	},
	/*OPP4 - OPP-SB*/
	{
		.enabled	= false,
		.freq		= 875000000,
		.u_volt		= 1310000,
	},
	/* Terminator */
	{
		.enabled	= 0,
		.freq		= 0,
		.u_volt		= 0,
	}
};

and if you thing it is line consuming, you can always define a "INIT" macro:
#define	OMAP_OPP_DEF(_enabled, _freq, _uv)	\
{						\
	.enabled	= _enabled,		\
	.freq		= _freq,		\
	.u_volt		= _uv,			\
}

and use it to initialize your array:

	/*OPP1 - OPP50*/
static struct omap_opp_def __initdata omap36xx_dsp_rate_table[] = {
	OMAP_OPP_DEF(true, 260000000, 930000),
	OMAP_OPP_DEF(true, 520000000, 1100000),
	OMAP_OPP_DEF(false, 660000000, 1260000),
	OMAP_OPP_DEF(false, 875000000, 1310000},
	OMAP_OPP_DEF(0, 0, 0),
};


Beside, although it should not hurt as they are not too big, these tables should
be compiled only if omap3630 is the target right?

>  struct omap_opp *omap3_mpu_rate_table;
>  struct omap_opp *omap3_dsp_rate_table;
>  struct omap_opp *omap3_l3_rate_table;
> @@ -1299,18 +1334,28 @@ static void __init configure_vc(void)
>  void __init omap3_pm_init_opp_table(void)
>  {
>  	int ret, i;
> +	struct omap_opp_def **omap3_opp_def_list;
>  	struct omap_opp_def *omap34xx_opp_def_list[] = {
>  		omap34xx_mpu_rate_table,
>  		omap34xx_l3_rate_table,
>  		omap34xx_dsp_rate_table
>  	};
> +	struct omap_opp_def *omap36xx_opp_def_list[] = {
> +		omap36xx_mpu_rate_table,
> +		omap36xx_l3_rate_table,
> +		omap36xx_dsp_rate_table
> +	};
>  	struct omap_opp **omap3_rate_tables[] = {
>  		&omap3_mpu_rate_table,
>  		&omap3_l3_rate_table,
>  		&omap3_dsp_rate_table
>  	};
> -	for (i = 0; i < ARRAY_SIZE(omap34xx_opp_def_list); i++) {
> -		ret = opp_init(omap3_rate_tables[i], omap34xx_opp_def_list[i]);
> +
> +	omap3_opp_def_list = cpu_is_omap3630() ? omap36xx_opp_def_list :
> +				omap34xx_opp_def_list;

here you cared for runtime target check, but tables above could be avoid if not omap3630.

> +
> +	for (i = 0; i < ARRAY_SIZE(omap3_rate_tables); i++) {
> +		ret = opp_init(omap3_rate_tables[i], omap3_opp_def_list[i]);
>  		/* We dont want half configured system at the moment */
>  		BUG_ON(ret);
>  	}
> -- 
> 1.6.3.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Eduardo Valentin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux