RE: [query] smartreflex: No PMIC hook to init smartreflex
-Original Message- From: Vishwanath Sripathy [mailto:vishwanath...@ti.com] Sent: Thursday, January 27, 2011 7:56 PM To: Menon, Nishanth; Premi, Sanjeev Cc: linux-omap@vger.kernel.org Subject: RE: [query] smartreflex: No PMIC hook to init smartreflex Nishant, -Original Message- From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap- ow...@vger.kernel.org] On Behalf Of Menon, Nishanth Sent: Thursday, January 27, 2011 3:06 PM To: Premi, Sanjeev Cc: linux-omap@vger.kernel.org Subject: Re: [query] smartreflex: No PMIC hook to init smartreflex Sanjeev, On Tue, Jan 25, 2011 at 20:55, Premi, Sanjeev pr...@ti.com wrote: While building the kernel at 2.6.37, i see this warning for omap3evm - with omap3630: Power Management for TI OMAP3. sr_init: No PMIC hook to init smartreflex -- THIS IS THE WARNING. smartreflex smartreflex.0: omap_sr_probe: SmartReflex driver initialized smartreflex smartreflex.1: omap_sr_probe: SmartReflex driver initialized SmartReflex Class3 initialized In the code, i see this comment: /* * sr_init is a late init. If by then a pmic specific API is not * registered either there is no need for anything to be done on * the PMIC side or somebody has forgotten to register a PMIC * handler. Warn for the second condition. */ if (sr_pmic_data sr_pmic_data-sr_pmic_init) sr_pmic_data-sr_pmic_init(); else pr_warning(%s: No PMIC hook to init smartreflex\n, __func__); But, I couldn't find any place where PMIC is being registered. This is a harmless warning (ideally, we should remove the pr_warning). the intent here is to have hook for pmic_init which could be populated for custom PMICs which may need something additional for Smart reflex enablement. if you look at the sr_pmic_data - it just has a single api for pmic_init e.g. in the case of TWL4030/5030, we might need to set the bit to switch mode from I2C1 to I2C_SR - e.g. the patch from Shweta[1] if Smartreflex AVS was the *only* mechanism in the system, we could have hooked pmic_init to this bit setting. but since the system can do voltage scaling (VP forceupdate/vc bypass) independent of SR AVS block, the patch in [1] does initialization independent of sr_pmic_data-pmic_init which makes sense. in short, my 2cents: the warning is probably something we should remove from the code. As you mentioned, incase of TWL4030/5030, we do not need any hook. However if some other PMIC is used that genuinely needs this hook, then shouldn't SR throw up this warning? As SR module is independent of PMIC, it cannot I possibly see different arguments from your description: 1) This is hook is not needed for TWL4030/5030. But still, still hook is needed for another PMIC. 2) It is okay to warn user for non-existent hook that is not really needed... in hope that there could be a different PMIC. 3) Despite saying that SR is independent of PMIC, we assume it as default and don't go thru the plugin route. Something seems to be missing in the argument. I haven't been able to spend more time on this since last mail (also reason for late response); but: 1) When user adds the hook - for different PMIC, this warning/info would go away. But not for current default systems. 2) By keeping TWL4030 as default, we are possibly not telling the person keen on replacing the PMIC what should be done? If SR is really PMIC independent, then warning is right only if TWL4030 is also 'plugged-in' as any other PMIC is expected to be. distinguish them. So I feel this warning should be present probably reworded better like No PMIC hook registered to init smartreflex. Either this PM IC does not need SR init or PMIC hook is missing. This wording - as is - would again be misleading. ~sanjeev Vishwa [1] http://marc.info/?l=linux-omapm=129584746102725w=2 Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [query] smartreflex: No PMIC hook to init smartreflex
Sanjeev, On Mon, Jan 31, 2011 at 9:50 PM, Premi, Sanjeev pr...@ti.com wrote: -Original Message- From: Vishwanath Sripathy [mailto:vishwanath...@ti.com] Sent: Thursday, January 27, 2011 7:56 PM To: Menon, Nishanth; Premi, Sanjeev Cc: linux-omap@vger.kernel.org Subject: RE: [query] smartreflex: No PMIC hook to init smartreflex Nishant, -Original Message- From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap- ow...@vger.kernel.org] On Behalf Of Menon, Nishanth Sent: Thursday, January 27, 2011 3:06 PM To: Premi, Sanjeev Cc: linux-omap@vger.kernel.org Subject: Re: [query] smartreflex: No PMIC hook to init smartreflex Sanjeev, On Tue, Jan 25, 2011 at 20:55, Premi, Sanjeev pr...@ti.com wrote: While building the kernel at 2.6.37, i see this warning for omap3evm - with omap3630: Power Management for TI OMAP3. sr_init: No PMIC hook to init smartreflex -- THIS IS THE WARNING. smartreflex smartreflex.0: omap_sr_probe: SmartReflex driver initialized smartreflex smartreflex.1: omap_sr_probe: SmartReflex driver initialized SmartReflex Class3 initialized In the code, i see this comment: /* * sr_init is a late init. If by then a pmic specific API is not * registered either there is no need for anything to be done on * the PMIC side or somebody has forgotten to register a PMIC * handler. Warn for the second condition. */ if (sr_pmic_data sr_pmic_data-sr_pmic_init) sr_pmic_data-sr_pmic_init(); else pr_warning(%s: No PMIC hook to init smartreflex\n, __func__); But, I couldn't find any place where PMIC is being registered. This is a harmless warning (ideally, we should remove the pr_warning). the intent here is to have hook for pmic_init which could be populated for custom PMICs which may need something additional for Smart reflex enablement. if you look at the sr_pmic_data - it just has a single api for pmic_init e.g. in the case of TWL4030/5030, we might need to set the bit to switch mode from I2C1 to I2C_SR - e.g. the patch from Shweta[1] if Smartreflex AVS was the *only* mechanism in the system, we could have hooked pmic_init to this bit setting. but since the system can do voltage scaling (VP forceupdate/vc bypass) independent of SR AVS block, the patch in [1] does initialization independent of sr_pmic_data-pmic_init which makes sense. in short, my 2cents: the warning is probably something we should remove from the code. As you mentioned, incase of TWL4030/5030, we do not need any hook. However if some other PMIC is used that genuinely needs this hook, then shouldn't SR throw up this warning? As SR module is independent of PMIC, it cannot I possibly see different arguments from your description: 1) This is hook is not needed for TWL4030/5030. But still, still hook is needed for another PMIC. True. struct 'sr_pmic_data' is generic way of handling all Pmic. The pmic needs to register to SR which it does by API 'sr_pmic_init'. Today this API just enables SR bit CFG_ENABLE_SRFLX in TWL PM module registers which is redundant in case of TWL chips as SR bit is already set in 'omap_twl_init' API in file omap_twl.c But in future if this API 'sr_pmic_init' has some other functionality or in case when Pmic is not TWL family chip then the initialization done by 'sr_pmic_init' is necessary. 2) It is okay to warn user for non-existent hook that is not really needed... in hope that there could be a different PMIC. 3) Despite saying that SR is independent of PMIC, we assume it as default and don't go thru the plugin route. Something seems to be missing in the argument. I haven't been able to spend more time on this since last mail (also reason for late response); but: 1) When user adds the hook - for different PMIC, this warning/info would go away. But not for current default systems. If some other PMIC is registered then ideally this warning shouldn't come and if it comes it hampers SR role but for TWL4030/TWL5030 this warning is harmless. 2) By keeping TWL4030 as default, we are possibly not telling the person keen on replacing the PMIC what should be done? If SR is really PMIC independent, then warning is right only if TWL4030 is also 'plugged-in' as any other PMIC is expected to be. distinguish them. So I feel this warning should be present probably reworded better like No PMIC hook registered to init smartreflex. Either this PM IC does not need SR init or PMIC hook is missing. This wording - as is - would again be misleading. ~sanjeev Vishwa [1] http://marc.info/?l=linux-omapm=129584746102725w=2 Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body
Re: [query] smartreflex: No PMIC hook to init smartreflex
Sanjeev, On Tue, Jan 25, 2011 at 20:55, Premi, Sanjeev pr...@ti.com wrote: While building the kernel at 2.6.37, i see this warning for omap3evm - with omap3630: Power Management for TI OMAP3. sr_init: No PMIC hook to init smartreflex -- THIS IS THE WARNING. smartreflex smartreflex.0: omap_sr_probe: SmartReflex driver initialized smartreflex smartreflex.1: omap_sr_probe: SmartReflex driver initialized SmartReflex Class3 initialized In the code, i see this comment: /* * sr_init is a late init. If by then a pmic specific API is not * registered either there is no need for anything to be done on * the PMIC side or somebody has forgotten to register a PMIC * handler. Warn for the second condition. */ if (sr_pmic_data sr_pmic_data-sr_pmic_init) sr_pmic_data-sr_pmic_init(); else pr_warning(%s: No PMIC hook to init smartreflex\n, __func__); But, I couldn't find any place where PMIC is being registered. This is a harmless warning (ideally, we should remove the pr_warning). the intent here is to have hook for pmic_init which could be populated for custom PMICs which may need something additional for Smart reflex enablement. if you look at the sr_pmic_data - it just has a single api for pmic_init e.g. in the case of TWL4030/5030, we might need to set the bit to switch mode from I2C1 to I2C_SR - e.g. the patch from Shweta[1] if Smartreflex AVS was the *only* mechanism in the system, we could have hooked pmic_init to this bit setting. but since the system can do voltage scaling (VP forceupdate/vc bypass) independent of SR AVS block, the patch in [1] does initialization independent of sr_pmic_data-pmic_init which makes sense. in short, my 2cents: the warning is probably something we should remove from the code. [1] http://marc.info/?l=linux-omapm=129584746102725w=2 Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [query] smartreflex: No PMIC hook to init smartreflex
Sanjeev, On Thu, Jan 27, 2011 at 3:05 PM, Menon, Nishanth n...@ti.com wrote: Sanjeev, On Tue, Jan 25, 2011 at 20:55, Premi, Sanjeev pr...@ti.com wrote: While building the kernel at 2.6.37, i see this warning for omap3evm - with omap3630: Power Management for TI OMAP3. sr_init: No PMIC hook to init smartreflex -- THIS IS THE WARNING. smartreflex smartreflex.0: omap_sr_probe: SmartReflex driver initialized smartreflex smartreflex.1: omap_sr_probe: SmartReflex driver initialized SmartReflex Class3 initialized In the code, i see this comment: /* * sr_init is a late init. If by then a pmic specific API is not * registered either there is no need for anything to be done on * the PMIC side or somebody has forgotten to register a PMIC * handler. Warn for the second condition. */ if (sr_pmic_data sr_pmic_data-sr_pmic_init) sr_pmic_data-sr_pmic_init(); else pr_warning(%s: No PMIC hook to init smartreflex\n, __func__); But, I couldn't find any place where PMIC is being registered. This is a harmless warning (ideally, we should remove the pr_warning). the intent here is to have hook for pmic_init which could be populated for custom PMICs which may need something additional for Smart reflex enablement. if you look at the sr_pmic_data - it just has a single api for pmic_init e.g. in the case of TWL4030/5030, we might need to set the bit to switch mode from I2C1 to I2C_SR - e.g. the patch from Shweta[1] if Smartreflex AVS was the *only* mechanism in the system, we could have hooked pmic_init to this bit setting. but since the system can do voltage scaling (VP forceupdate/vc bypass) independent of SR AVS block, the patch in [1] does initialization independent of sr_pmic_data-pmic_init which makes sense. in short, my 2cents: the warning is probably something we should remove from the code. [1] http://marc.info/?l=linux-omapm=129584746102725w=2 Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html I have tested on OMAP4 board also, Smart reflex works fine. It brings down the nominal voltages to optimum levels, the registration done by generic Pmic framework to SR wouldn't hamper SR woking as the SR bit setting is done specifically in Triton specific file 'omap_twl.c' and thus, that warning is harmless. The patch you should take for SR to work is: https://patchwork.kernel.org/patch/510971/ -- Thanks, Regards, Shweta -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [query] smartreflex: No PMIC hook to init smartreflex
Nishant, -Original Message- From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap- ow...@vger.kernel.org] On Behalf Of Menon, Nishanth Sent: Thursday, January 27, 2011 3:06 PM To: Premi, Sanjeev Cc: linux-omap@vger.kernel.org Subject: Re: [query] smartreflex: No PMIC hook to init smartreflex Sanjeev, On Tue, Jan 25, 2011 at 20:55, Premi, Sanjeev pr...@ti.com wrote: While building the kernel at 2.6.37, i see this warning for omap3evm - with omap3630: Power Management for TI OMAP3. sr_init: No PMIC hook to init smartreflex -- THIS IS THE WARNING. smartreflex smartreflex.0: omap_sr_probe: SmartReflex driver initialized smartreflex smartreflex.1: omap_sr_probe: SmartReflex driver initialized SmartReflex Class3 initialized In the code, i see this comment: /* * sr_init is a late init. If by then a pmic specific API is not * registered either there is no need for anything to be done on * the PMIC side or somebody has forgotten to register a PMIC * handler. Warn for the second condition. */ if (sr_pmic_data sr_pmic_data-sr_pmic_init) sr_pmic_data-sr_pmic_init(); else pr_warning(%s: No PMIC hook to init smartreflex\n, __func__); But, I couldn't find any place where PMIC is being registered. This is a harmless warning (ideally, we should remove the pr_warning). the intent here is to have hook for pmic_init which could be populated for custom PMICs which may need something additional for Smart reflex enablement. if you look at the sr_pmic_data - it just has a single api for pmic_init e.g. in the case of TWL4030/5030, we might need to set the bit to switch mode from I2C1 to I2C_SR - e.g. the patch from Shweta[1] if Smartreflex AVS was the *only* mechanism in the system, we could have hooked pmic_init to this bit setting. but since the system can do voltage scaling (VP forceupdate/vc bypass) independent of SR AVS block, the patch in [1] does initialization independent of sr_pmic_data-pmic_init which makes sense. in short, my 2cents: the warning is probably something we should remove from the code. As you mentioned, incase of TWL4030/5030, we do not need any hook. However if some other PMIC is used that genuinely needs this hook, then shouldn't SR throw up this warning? As SR module is independent of PMIC, it cannot distinguish them. So I feel this warning should be present probably reworded better like No PMIC hook registered to init smartreflex. Either this PM IC does not need SR init or PMIC hook is missing. Vishwa [1] http://marc.info/?l=linux-omapm=129584746102725w=2 Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [query] smartreflex: No PMIC hook to init smartreflex
Vishwanath Sripathy wrote, on 01/27/2011 07:55 PM: Nishant, -Original Message- From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap- ow...@vger.kernel.org] On Behalf Of Menon, Nishanth Sent: Thursday, January 27, 2011 3:06 PM To: Premi, Sanjeev Cc: linux-omap@vger.kernel.org Subject: Re: [query] smartreflex: No PMIC hook to init smartreflex Sanjeev, On Tue, Jan 25, 2011 at 20:55, Premi, Sanjeevpr...@ti.com wrote: While building the kernel at 2.6.37, i see this warning for omap3evm - with omap3630: Power Management for TI OMAP3. sr_init: No PMIC hook to init smartreflex-- THIS IS THE WARNING. smartreflex smartreflex.0: omap_sr_probe: SmartReflex driver initialized smartreflex smartreflex.1: omap_sr_probe: SmartReflex driver initialized SmartReflex Class3 initialized In the code, i see this comment: /* * sr_init is a late init. If by then a pmic specific API is not * registered either there is no need for anything to be done on * the PMIC side or somebody has forgotten to register a PMIC * handler. Warn for the second condition. */ if (sr_pmic_data sr_pmic_data-sr_pmic_init) sr_pmic_data-sr_pmic_init(); else pr_warning(%s: No PMIC hook to init smartreflex\n, __func__); But, I couldn't find any place where PMIC is being registered. This is a harmless warning (ideally, we should remove the pr_warning). the intent here is to have hook for pmic_init which could be populated for custom PMICs which may need something additional for Smart reflex enablement. if you look at the sr_pmic_data - it just has a single api for pmic_init e.g. in the case of TWL4030/5030, we might need to set the bit to switch mode from I2C1 to I2C_SR - e.g. the patch from Shweta[1] if Smartreflex AVS was the *only* mechanism in the system, we could have hooked pmic_init to this bit setting. but since the system can do voltage scaling (VP forceupdate/vc bypass) independent of SR AVS block, the patch in [1] does initialization independent of sr_pmic_data-pmic_init which makes sense. in short, my 2cents: the warning is probably something we should remove from the code. As you mentioned, incase of TWL4030/5030, we do not need any hook. However if some other PMIC is used that genuinely needs this hook, then shouldn't SR throw up this warning? As SR module is independent of PMIC, it cannot distinguish them. So I feel this warning should be present probably reworded better like No PMIC hook registered to init smartreflex. Either this PM IC does not need SR init or PMIC hook is missing. Fair enough - but how do we know if the warning spawns off question such as this thread? aka false alarm? if a platform does need initialization it should be up to the platform porting person rt? Just that I dont think it makes sense to false flag folks considering that the API is optional. -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
