>>-----Original Message----- >>From: Kevin Hilman [mailto:khilman@xxxxxxxxxxxxxxxxxxx] >>Sent: Thursday, November 11, 2010 12:52 AM >>To: Gopinath, Thara >>Cc: linux-omap@xxxxxxxxxxxxxxx; paul@xxxxxxxxx; Cousson, Benoit; Sripathy, >>Vishwanath; Sawant, Anand >>Subject: Re: [PATCH v3 2/6] OMAP4: Adding voltage driver support >> >>Thara Gopinath <thara@xxxxxx> writes: >> >>> OMAP4 has three scalable voltage domains vdd_mpu, vdd_iva >>> and vdd_core. This patch adds the voltage tables and other >>> configurable voltage processor and voltage controller >>> settings to control these three scalable domains in OMAP4. >>> >>> Signed-off-by: Thara Gopinath <thara@xxxxxx> >> >>[...] >> >>> +/* >>> + * Structures containing OMAP4430 voltage supported and various >>> + * data associated with it per voltage domain basis. Smartreflex Ntarget >>> + * values are left as 0 as they have to be populated by smartreflex >>> + * driver after reading the efuse. >>> + */ >>> +static struct omap_volt_data omap44xx_vdd_mpu_volt_data[] = { >>> + {.volt_nominal = 930000, .sr_errminlimit = 0xF4, .vp_errgain = 0x0C}, >>> + {.volt_nominal = 1100000, .sr_errminlimit = 0xF9, .vp_errgain = 0x16}, >>> + {.volt_nominal = 1260000, .sr_errminlimit = 0xFA, .vp_errgain = 0x23}, >>> + {.volt_nominal = 1350000, .sr_errminlimit = 0xFA, .vp_errgain = 0x27}, >>> +}; >>> + >>> +static struct omap_volt_data omap44xx_vdd_iva_volt_data[] = { >>> + {.volt_nominal = 930000, .sr_errminlimit = 0xF4, .vp_errgain = 0x0C}, >>> + {.volt_nominal = 1100000, .sr_errminlimit = 0xF9, .vp_errgain = 0x16}, >>> + {.volt_nominal = 1260000, .sr_errminlimit = 0xFA, .vp_errgain = 0x23}, >>> +}; >>> + >>> +static struct omap_volt_data omap44xx_vdd_core_volt_data[] = { >>> + {.volt_nominal = 930000, .sr_errminlimit = 0xF4, .vp_errgain = 0x0C}, >>> + {.volt_nominal = 1100000, .sr_errminlimit = 0xF9, .vp_errgain = 0x16}, >>> +}; >> >>As mentioned in previous reviews, the standard is to write hex value >>using lower case letters. Please fix this up in the both OMAP3 & 4 series. Ok. Sorry for missing this. >> >>[...] >> >>> +/* Sets up all the VDD related info for OMAP4 */ >>> +static void __init omap4_vdd_data_configure(struct omap_vdd_info *vdd) >>> +{ >>> + struct clk *sys_ck; >>> + u32 sys_clk_speed, timeout_val, waittime; >>> + >>> + if (!strcmp(vdd->voltdm.name, "mpu")) { >>> + vdd->vp_reg.vlimitto_vddmin = OMAP4_VP_MPU_VLIMITTO_VDDMIN; >>> + vdd->vp_reg.vlimitto_vddmax = OMAP4_VP_MPU_VLIMITTO_VDDMAX; >>> + vdd->volt_data = omap44xx_vdd_mpu_volt_data; >>> + vdd->volt_data_count = ARRAY_SIZE(omap44xx_vdd_mpu_volt_data); >>> + vdd->vp_reg.tranxdone_status = >>> + OMAP4430_VP_MPU_TRANXDONE_ST_MASK; >>> + vdd->cmdval_reg = OMAP4_PRM_VC_VAL_CMD_VDD_MPU_L_OFFSET; >>> + vdd->vdd_sr_reg = OMAP4_VDD_MPU_SR_VOLT_REG; >>> + } else if (!strcmp(vdd->voltdm.name, "core")) { >>> + vdd->vp_reg.vlimitto_vddmin = OMAP4_VP_CORE_VLIMITTO_VDDMIN; >>> + vdd->vp_reg.vlimitto_vddmax = OMAP4_VP_CORE_VLIMITTO_VDDMAX; >>> + vdd->volt_data = omap44xx_vdd_core_volt_data; >>> + vdd->volt_data_count = ARRAY_SIZE(omap44xx_vdd_core_volt_data); >>> + vdd->vp_reg.tranxdone_status = >>> + OMAP4430_VP_CORE_TRANXDONE_ST_MASK; >>> + vdd->cmdval_reg = OMAP4_PRM_VC_VAL_CMD_VDD_CORE_L_OFFSET; >>> + vdd->vdd_sr_reg = OMAP4_VDD_CORE_SR_VOLT_REG; >>> + } else if (!strcmp(vdd->voltdm.name, "iva")) { >>> + vdd->vp_reg.vlimitto_vddmin = OMAP4_VP_IVA_VLIMITTO_VDDMIN; >>> + vdd->vp_reg.vlimitto_vddmax = OMAP4_VP_IVA_VLIMITTO_VDDMAX; >>> + vdd->volt_data = omap44xx_vdd_iva_volt_data; >>> + vdd->volt_data_count = ARRAY_SIZE(omap44xx_vdd_iva_volt_data); >>> + vdd->vp_reg.tranxdone_status = >>> + OMAP4430_VP_IVA_TRANXDONE_ST_MASK; >>> + vdd->cmdval_reg = OMAP4_PRM_VC_VAL_CMD_VDD_IVA_L_OFFSET; >>> + vdd->vdd_sr_reg = OMAP4_VDD_IVA_SR_VOLT_REG; >>> + } else { >>> + pr_warning("%s: vdd_%s does not exisit in OMAP4\n", >>> + __func__, vdd->voltdm.name); >>> + return; >>> + } >>> + >>> + /* >>> + * Sys clk rate is require to calculate vp timeout value and >>> + * smpswaittimemin and smpswaittimemax. >>> + */ >>> + sys_ck = clk_get(NULL, "sys_clkin_ck"); >>> + if (IS_ERR(sys_ck)) { >>> + pr_warning("%s: Could not get the sys clk to calculate" >>> + "various vdd_%s params\n", __func__, vdd->voltdm.name); >>> + return; >>> + } >>> + sys_clk_speed = clk_get_rate(sys_ck); >>> + clk_put(sys_ck); >>> + >>> + /* Divide to avoid overflow */ >>> + sys_clk_speed /= 1000; >>> + >>> + /* Nominal/Reset voltage of the VDD */ >>> + vdd->nominal_volt = vdd->curr_volt = 1200000; >> >>same comment as from OMAP3 series. Will take care >> >>[...] >> >>> /* Generic voltage init functions */ >>> static void __init init_voltageprocessor(struct omap_vdd_info *vdd) >>> { >>> @@ -753,6 +945,14 @@ static int vp_forceupdate_scale_voltage(struct >>omap_vdd_info *vdd, >>> vc_cmd_on_mask = OMAP3430_VC_CMD_ON_MASK; >>> prm_irqst_reg_offs = OMAP3_PRM_IRQSTATUS_MPU_OFFSET; >>> ocp_mod = OCP_MOD; >>> + } else if (cpu_is_omap44xx()) { >>> + vc_cmd_on_shift = OMAP4430_ON_SHIFT; >>> + vc_cmd_on_mask = OMAP4430_ON_MASK; >>> + if (!strcmp(vdd->voltdm.name, "mpu")) >>> + prm_irqst_reg_offs = OMAP4_PRM_IRQSTATUS_MPU_2_OFFSET; >>> + else >>> + prm_irqst_reg_offs = OMAP4_PRM_IRQSTATUS_MPU_OFFSET; >>> + ocp_mod = OMAP4430_PRM_OCP_SOCKET_MOD; >> >>again, please no cpu_is_* outside of init functions. I had a concern regarding this explained in my reply to your omap3 review. [...] Regards Thara -- 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
