RE: [PATCH v4 7/9] OMAP3: PM: Adding debug support to Voltage and Smartreflex drivers
-Original Message- From: Nishanth Menon [mailto:n...@ti.com] Sent: Wednesday, December 08, 2010 10:05 PM To: Gopinath, Thara Cc: Kevin Hilman; linux-omap@vger.kernel.org; p...@pwsan.com; Cousson, Benoit; Sripathy, Vishwanath; Sawant, Anand Subject: Re: [PATCH v4 7/9] OMAP3: PM: Adding debug support to Voltage and Smartreflex drivers Gopinath, Thara had written, on 12/08/2010 10:18 AM, the following: [..] And, AFAICT, it wasn't clear from the current code or docs whether this could work or was expected to work either, e.g., if you set override_volt_params back to zero, to the original values all get reused? If you want to provide this feature, then it should be documented and made clear that this is an intended goal. Thinking about this more, the main thing I don't like about this approach is that the active code paths (enable disable) have to check each time if any of these values have been overidden. Rather than have several places in the active code paths where this override value is checked, there the sysfs methods should simply update the values that are used by the core code. This way, the core would not need to know about where the values came from (defalts, volt_data, user override, etc.) If you want to provide a way to revert this, then maybe writing -1 will should switch that value back to the HW default, or volt_data default. Kevin, Benoit, Nishant et al, Without this override flag today there is no direct way of allowing user to write into these parameters. My question is, Glancing at the debug entries being overidden, as developer (debug users) working for tweaking parameters for their platform - yes - we will need some mechanism to runtime tweak and see the behavior without needing to rebuild the kernel everytime. On production system (OS users): they should'nt be using this. is there a need for the parameters to be over-written from the user-space? If yes, I need ideas on how to implement it with using override_volt_params ! Lets get the basics in kernel.org in some form! IMHO, all this double knobs are un-necessary overheads in codeflow for development only code- just provide the debugfs entries that reflect the data in their original structures, use the original structures everytime we go to a new transition (aka if you change the params in debugfs, they dont take effect till you do another transition).. but that is just my 2cents. Nishant, The issue here is most of these parameters are one time setting (during init) and need not be changed at all if the user does not wish to over-ride them for debug purpose. Hence the need for the checks (not double-hooks). But I agree with your point that let us get the basic in the kernel.org in some form. So for this first version that we plan to push to kernel.org, I plan to expose out these parameters to user space but not allow a write access to them. The write access part can be added later whenever required. Regards Thara --- 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: [PATCH v4 7/9] OMAP3: PM: Adding debug support to Voltage and Smartreflex drivers
Gopinath, Thara wrote, on 12/09/2010 03:43 AM: -Original Message- From: Nishanth Menon [mailto:n...@ti.com] Sent: Wednesday, December 08, 2010 10:05 PM To: Gopinath, Thara Cc: Kevin Hilman; linux-omap@vger.kernel.org; p...@pwsan.com; Cousson, Benoit; Sripathy, Vishwanath; Sawant, Anand Subject: Re: [PATCH v4 7/9] OMAP3: PM: Adding debug support to Voltage and Smartreflex drivers Gopinath, Thara had written, on 12/08/2010 10:18 AM, the following: [..] And, AFAICT, it wasn't clear from the current code or docs whether this could work or was expected to work either, e.g., if you set override_volt_params back to zero, to the original values all get reused? If you want to provide this feature, then it should be documented and made clear that this is an intended goal. Thinking about this more, the main thing I don't like about this approach is that the active code paths (enable disable) have to check each time if any of these values have been overidden. Rather than have several places in the active code paths where this override value is checked, there the sysfs methods should simply update the values that are used by the core code. This way, the core would not need to know about where the values came from (defalts, volt_data, user override, etc.) If you want to provide a way to revert this, then maybe writing -1 will should switch that value back to the HW default, or volt_data default. Kevin, Benoit, Nishant et al, Without this override flag today there is no direct way of allowing user to write into these parameters. My question is, Glancing at the debug entries being overidden, as developer (debug users) working for tweaking parameters for their platform - yes - we will need some mechanism to runtime tweak and see the behavior without needing to rebuild the kernel everytime. On production system (OS users): they should'nt be using this. is there a need for the parameters to be over-written from the user-space? If yes, I need ideas on how to implement it with using override_volt_params ! Lets get the basics in kernel.org in some form! IMHO, all this double knobs are un-necessary overheads in codeflow for development only code- just provide the debugfs entries that reflect the data in their original structures, use the original structures everytime we go to a new transition (aka if you change the params in debugfs, they dont take effect till you do another transition).. but that is just my 2cents. Nishant, The issue here is most of these parameters are one time setting (during init) and need not be changed at all if the user does not wish to over-ride them for debug purpose. Hence the need for the checks (not double-hooks). But I agree If they are init time thingy, then why not set it up so that when some one echo's to the debugfs, it writes to the register as well? That way you dont need to add runtime check and the debug developer does'nt need to worry about echo 1 magic_control_sys_fs_file (just kidding). with your point that let us get the basic in the kernel.org in some form. So for this first version that we plan to push to kernel.org, I plan to expose out these parameters to user space but not allow a write access to them. The write access part can be added later whenever required. Sure.. at this point, anything that actually works is welcome :) -- 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: [PATCH v4 7/9] OMAP3: PM: Adding debug support to Voltage and Smartreflex drivers
-Original Message- From: Menon, Nishanth Sent: Thursday, December 09, 2010 4:54 PM To: Gopinath, Thara Cc: Kevin Hilman; linux-omap@vger.kernel.org; p...@pwsan.com; Cousson, Benoit; Sripathy, Vishwanath; Sawant, Anand Subject: Re: [PATCH v4 7/9] OMAP3: PM: Adding debug support to Voltage and Smartreflex drivers Gopinath, Thara wrote, on 12/09/2010 03:43 AM: -Original Message- From: Nishanth Menon [mailto:n...@ti.com] Sent: Wednesday, December 08, 2010 10:05 PM To: Gopinath, Thara Cc: Kevin Hilman; linux-omap@vger.kernel.org; p...@pwsan.com; Cousson, Benoit; Sripathy, Vishwanath; Sawant, Anand Subject: Re: [PATCH v4 7/9] OMAP3: PM: Adding debug support to Voltage and Smartreflex drivers Gopinath, Thara had written, on 12/08/2010 10:18 AM, the following: [..] And, AFAICT, it wasn't clear from the current code or docs whether this could work or was expected to work either, e.g., if you set override_volt_params back to zero, to the original values all get reused? If you want to provide this feature, then it should be documented and made clear that this is an intended goal. Thinking about this more, the main thing I don't like about this approach is that the active code paths (enable disable) have to check each time if any of these values have been overidden. Rather than have several places in the active code paths where this override value is checked, there the sysfs methods should simply update the values that are used by the core code. This way, the core would not need to know about where the values came from (defalts, volt_data, user override, etc.) If you want to provide a way to revert this, then maybe writing -1 will should switch that value back to the HW default, or volt_data default. Kevin, Benoit, Nishant et al, Without this override flag today there is no direct way of allowing user to write into these parameters. My question is, Glancing at the debug entries being overidden, as developer (debug users) working for tweaking parameters for their platform - yes - we will need some mechanism to runtime tweak and see the behavior without needing to rebuild the kernel everytime. On production system (OS users): they should'nt be using this. is there a need for the parameters to be over-written from the user-space? If yes, I need ideas on how to implement it with using override_volt_params ! Lets get the basics in kernel.org in some form! IMHO, all this double knobs are un-necessary overheads in codeflow for development only code- just provide the debugfs entries that reflect the data in their original structures, use the original structures everytime we go to a new transition (aka if you change the params in debugfs, they dont take effect till you do another transition).. but that is just my 2cents. Nishant, The issue here is most of these parameters are one time setting (during init) and need not be changed at all if the user does not wish to over-ride them for debug purpose. Hence the need for the checks (not double-hooks). But I agree If they are init time thingy, then why not set it up so that when some one echo's to the debugfs, it writes to the register as well? That way you dont need to add runtime check and the debug developer does'nt need to worry about echo 1 magic_control_sys_fs_file (just kidding). Yes but then for it the debugfs set get APIs should have access to the data structures which stores the register offset etc. Today I am using a single API to set any parameter any vdd. This will have to change. Maybe one API per parameter. I cannot currently think of any better solution :-)! with your point that let us get the basic in the kernel.org in some form. So for this first version that we plan to push to kernel.org, I plan to expose out these parameters to user space but not allow a write access to them. The write access part can be added later whenever required. Sure.. at this point, anything that actually works is welcome :) Ok :-) ! Regards Thara -- 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: [PATCH v4 7/9] OMAP3: PM: Adding debug support to Voltage and Smartreflex drivers
Gopinath, Thara th...@ti.com writes: [...] So for this first version that we plan to push to kernel.org, I plan to expose out these parameters to user space but not allow a write access to them. The write access part can be added later whenever required. OK with me. Kevin -- 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: [PATCH v4 7/9] OMAP3: PM: Adding debug support to Voltage and Smartreflex drivers
Kevin Hilman had written, on 12/09/2010 11:19 AM, the following: Gopinath, Thara th...@ti.com writes: [...] So for this first version that we plan to push to kernel.org, I plan to expose out these parameters to user space but not allow a write access to them. The write access part can be added later whenever required. OK with me. +1 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: [PATCH v4 7/9] OMAP3: PM: Adding debug support to Voltage and Smartreflex drivers
-Original Message- From: Kevin Hilman [mailto:khil...@deeprootsystems.com] Sent: Tuesday, November 16, 2010 3:43 AM To: Gopinath, Thara Cc: linux-omap@vger.kernel.org; p...@pwsan.com; Cousson, Benoit; Sripathy, Vishwanath; Sawant, Anand Subject: Re: [PATCH v4 7/9] OMAP3: PM: Adding debug support to Voltage and Smartreflex drivers Gopinath, Thara th...@ti.com writes: -Original Message- From: Kevin Hilman [mailto:khil...@deeprootsystems.com] Sent: Thursday, November 11, 2010 12:43 AM To: Gopinath, Thara Cc: linux-omap@vger.kernel.org; p...@pwsan.com; Cousson, Benoit; Sripathy, Vishwanath; Sawant, Anand Subject: Re: [PATCH v4 7/9] OMAP3: PM: Adding debug support to Voltage and Smartreflex drivers Thara Gopinath th...@ti.com writes: This patch adds debug support to the voltage and smartreflex drivers. This means a whole bunch of voltage processor and smartreflex parameters are now visible through the pm debugfs. The voltage parameters can be viewed at /debug/voltage/vdd_x/parameter and the smartreflex parameters can be viewed at /debug/vdd_x/smartreflex/parameter To enable overriding of these parameters from user side, write 1 into /debug/voltage/vdd_x/override_volt_params Please just git rid of any sort of override parameter from sysfs. Instead, you can detect in the sysfs code itself if any parameters were changed and then set the vdd-user_override flag. But in the sys-fs code I do not have access to vdd. How do I then set this flag? But when will I unset this flag?? You can't. And, AFAICT, it wasn't clear from the current code or docs whether this could work or was expected to work either, e.g., if you set override_volt_params back to zero, to the original values all get reused? If you want to provide this feature, then it should be documented and made clear that this is an intended goal. Thinking about this more, the main thing I don't like about this approach is that the active code paths (enable disable) have to check each time if any of these values have been overidden. Rather than have several places in the active code paths where this override value is checked, there the sysfs methods should simply update the values that are used by the core code. This way, the core would not need to know about where the values came from (defalts, volt_data, user override, etc.) If you want to provide a way to revert this, then maybe writing -1 will should switch that value back to the HW default, or volt_data default. Kevin, Benoit, Nishant et al, Without this override flag today there is no direct way of allowing user to write into these parameters. My question is, is there a need for the parameters to be over-written from the user-space? If yes, I need ideas on how to implement it with using override_volt_params ! Regards Thara -- 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: [PATCH v4 7/9] OMAP3: PM: Adding debug support to Voltage and Smartreflex drivers
Gopinath, Thara had written, on 12/08/2010 10:18 AM, the following: [..] And, AFAICT, it wasn't clear from the current code or docs whether this could work or was expected to work either, e.g., if you set override_volt_params back to zero, to the original values all get reused? If you want to provide this feature, then it should be documented and made clear that this is an intended goal. Thinking about this more, the main thing I don't like about this approach is that the active code paths (enable disable) have to check each time if any of these values have been overidden. Rather than have several places in the active code paths where this override value is checked, there the sysfs methods should simply update the values that are used by the core code. This way, the core would not need to know about where the values came from (defalts, volt_data, user override, etc.) If you want to provide a way to revert this, then maybe writing -1 will should switch that value back to the HW default, or volt_data default. Kevin, Benoit, Nishant et al, Without this override flag today there is no direct way of allowing user to write into these parameters. My question is, Glancing at the debug entries being overidden, as developer (debug users) working for tweaking parameters for their platform - yes - we will need some mechanism to runtime tweak and see the behavior without needing to rebuild the kernel everytime. On production system (OS users): they should'nt be using this. is there a need for the parameters to be over-written from the user-space? If yes, I need ideas on how to implement it with using override_volt_params ! Lets get the basics in kernel.org in some form! IMHO, all this double knobs are un-necessary overheads in codeflow for development only code- just provide the debugfs entries that reflect the data in their original structures, use the original structures everytime we go to a new transition (aka if you change the params in debugfs, they dont take effect till you do another transition).. but that is just my 2cents. --- 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: [PATCH v4 7/9] OMAP3: PM: Adding debug support to Voltage and Smartreflex drivers
-Original Message- From: Kevin Hilman [mailto:khil...@deeprootsystems.com] Sent: Thursday, November 11, 2010 12:43 AM To: Gopinath, Thara Cc: linux-omap@vger.kernel.org; p...@pwsan.com; Cousson, Benoit; Sripathy, Vishwanath; Sawant, Anand Subject: Re: [PATCH v4 7/9] OMAP3: PM: Adding debug support to Voltage and Smartreflex drivers Thara Gopinath th...@ti.com writes: This patch adds debug support to the voltage and smartreflex drivers. This means a whole bunch of voltage processor and smartreflex parameters are now visible through the pm debugfs. The voltage parameters can be viewed at /debug/voltage/vdd_x/parameter and the smartreflex parameters can be viewed at /debug/vdd_x/smartreflex/parameter To enable overriding of these parameters from user side, write 1 into /debug/voltage/vdd_x/override_volt_params Please just git rid of any sort of override parameter from sysfs. Instead, you can detect in the sysfs code itself if any parameters were changed and then set the vdd-user_override flag. But when will I unset this flag?? Regards Thara -- 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: [PATCH v4 7/9] OMAP3: PM: Adding debug support to Voltage and Smartreflex drivers
Gopinath, Thara th...@ti.com writes: -Original Message- From: Kevin Hilman [mailto:khil...@deeprootsystems.com] Sent: Thursday, November 11, 2010 12:43 AM To: Gopinath, Thara Cc: linux-omap@vger.kernel.org; p...@pwsan.com; Cousson, Benoit; Sripathy, Vishwanath; Sawant, Anand Subject: Re: [PATCH v4 7/9] OMAP3: PM: Adding debug support to Voltage and Smartreflex drivers Thara Gopinath th...@ti.com writes: This patch adds debug support to the voltage and smartreflex drivers. This means a whole bunch of voltage processor and smartreflex parameters are now visible through the pm debugfs. The voltage parameters can be viewed at /debug/voltage/vdd_x/parameter and the smartreflex parameters can be viewed at /debug/vdd_x/smartreflex/parameter To enable overriding of these parameters from user side, write 1 into /debug/voltage/vdd_x/override_volt_params Please just git rid of any sort of override parameter from sysfs. Instead, you can detect in the sysfs code itself if any parameters were changed and then set the vdd-user_override flag. But when will I unset this flag?? You can't. And, AFAICT, it wasn't clear from the current code or docs whether this could work or was expected to work either, e.g., if you set override_volt_params back to zero, to the original values all get reused? If you want to provide this feature, then it should be documented and made clear that this is an intended goal. Thinking about this more, the main thing I don't like about this approach is that the active code paths (enable disable) have to check each time if any of these values have been overidden. Rather than have several places in the active code paths where this override value is checked, there the sysfs methods should simply update the values that are used by the core code. This way, the core would not need to know about where the values came from (defalts, volt_data, user override, etc.) If you want to provide a way to revert this, then maybe writing -1 will should switch that value back to the HW default, or volt_data default. Kevin -- 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: [PATCH v4 7/9] OMAP3: PM: Adding debug support to Voltage and Smartreflex drivers
Thara Gopinath th...@ti.com writes: This patch adds debug support to the voltage and smartreflex drivers. This means a whole bunch of voltage processor and smartreflex parameters are now visible through the pm debugfs. The voltage parameters can be viewed at /debug/voltage/vdd_x/parameter and the smartreflex parameters can be viewed at /debug/vdd_x/smartreflex/parameter To enable overriding of these parameters from user side, write 1 into /debug/voltage/vdd_x/override_volt_params Please just git rid of any sort of override parameter from sysfs. Instead, you can detect in the sysfs code itself if any parameters were changed and then set the vdd-user_override flag. Kevin -- 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
