| Message ID | 1362396957-30113-4-git-send-email-hvaibhav@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, * hvaibhav@ti.com <hvaibhav@ti.com> [130304 03:40]: > From: Vaibhav Hiremath <hvaibhav@ti.com> > > Currently there is no clean mechanism to control debugSS module and > you have to always keep clocks enabled, either, > > - By enabling it during boot as part of clk_init function. > Or > - By having HWMOD_INIT_NO_IDLE flag in hwmod data. > > Based on the discussion, > http://www.mail-archive.com/linux-omap@vger.kernel.org/msg81771.html > > This patch introduces new kernel parameter "omap_debugss_en", > which will allow user to control debugSS module enable/disable > part during boot-time. I suggest you just make this part into a standard DT only device driver. That way the command line parsing and clock enabling can happen the normal way. Is there any reason why this could not be a loadable module? Regards, Tony > Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com> > Cc: Kevin Hilman <khilman@linaro.org> > Cc: Paul Walmsley <paul@pwsan.com> > Cc: Tony Lindgren <tony@atomide.com> > --- > Tested on > - AM335x based EVM and BeagleBone platforms > > Documentation/kernel-parameters.txt | 3 + > arch/arm/mach-omap2/Makefile | 2 +- > arch/arm/mach-omap2/debugss.c | 80 +++++++++++++++++++++++++++++++++++ > 3 files changed, 84 insertions(+), 1 deletions(-) > create mode 100644 arch/arm/mach-omap2/debugss.c > > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt > index 6c72381..bf1c822 100644 > --- a/Documentation/kernel-parameters.txt > +++ b/Documentation/kernel-parameters.txt > @@ -2051,6 +2051,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted. > For example, to override I2C bus2: > omap_mux=i2c2_scl.i2c2_scl=0x100,i2c2_sda.i2c2_sda=0x100 > > + omap_debugss_en [OMAP] Enable Debug Sub-System module required > + for JTAG debugging. > + > oprofile.timer= [HW] > Use timer interrupt instead of performance counters > > diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile > index d1156cf..aaf5cc2 100644 > --- a/arch/arm/mach-omap2/Makefile > +++ b/arch/arm/mach-omap2/Makefile > @@ -5,7 +5,7 @@ > # Common support > obj-y := id.o io.o control.o mux.o devices.o fb.o serial.o gpmc.o timer.o pm.o \ > common.o gpio.o dma.o wd_timer.o display.o i2c.o hdq1w.o omap_hwmod.o \ > - omap_device.o sram.o > + omap_device.o sram.o debugss.o > > omap-2-3-common = irq.o > hwmod-common = omap_hwmod.o \ > diff --git a/arch/arm/mach-omap2/debugss.c b/arch/arm/mach-omap2/debugss.c > new file mode 100644 > index 0000000..b45bf2c > --- /dev/null > +++ b/arch/arm/mach-omap2/debugss.c > @@ -0,0 +1,80 @@ > +/* > + * debugss.c: Debug Sus-System related code goes in here > + * > + * Copyright (C) {2013} Texas Instruments Incorporated - http://www.ti.com/ > + * > + * This file is automatically generated from the AM33XX hardware databases. > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License as > + * published by the Free Software Foundation version 2. > + * > + * This program is distributed "as is" WITHOUT ANY WARRANTY of any > + * kind, whether express or implied; without even the implied warranty > + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include <linux/init.h> > +#include <linux/err.h> > +#include <linux/clk.h> > + > +#include "omap_hwmod.h" > + > +static bool is_debugss_en; > + > +/** > + * omap_debugss_en - Enable debugss clock/module based on user config > + * > + * During kernel bootup, omap2 hwmod framework will disable all the > + * unused/unclaimed modules, which in turn also disables debugss module. > + * This breaks any further debugging capability provided by HW. > + * > + * > + * Introduce early param which allows user to enable clock/module - > + * > + * omap_debugss_en (For all OMAP2 architectures) > + * > + * Please note that, with this command-line param, module always remain > + * enabled. > + */ > +static int __init omap_debugss_en(char *str) > +{ > + is_debugss_en = true; > + return 0; > +} > +early_param("omap_debugss_en", omap_debugss_en); > + > +static int __init _omap2_debugss_enable(void) > +{ > + const char oh_name[10] = "debugss"; > + struct omap_hwmod *oh; > + int ret; > + > + if (is_debugss_en) { > + struct omap_hwmod_opt_clk *oc; > + int i; > + > + oh = omap_hwmod_lookup(oh_name); > + if (!oh) { > + pr_err("debugss device not found\n"); > + return 0; > + } > + > + /* Make sure that hwmod internal data structures are setup */ > + ret = omap_hwmod_setup_one(oh_name); > + if (ret) { > + pr_err("failed to setup hwmod for %s\n", oh_name); > + return 0; > + } > + /* Enable optional clocks */ > + for (i = oh->opt_clks_cnt, oc = oh->opt_clks; i > 0; i--, oc++) { > + if (oc->_clk) > + clk_prepare_enable(oc->_clk); > + } > + /* Enable debugss clock/module */ > + omap_hwmod_enable(oh); > + } > + > + return 0; > +} > +device_initcall(_omap2_debugss_enable); > -- > 1.7.0.4 >
> -----Original Message----- > From: Tony Lindgren [mailto:tony@atomide.com] > Sent: Monday, April 08, 2013 11:00 PM > To: Hiremath, Vaibhav > Cc: linux-omap@vger.kernel.org; khilman@linaro.org; paul@pwsan.com; > Nayak, Rajendra; linux-arm-kernel@lists.infradead.org > Subject: Re: [RFC PATCH 3/3] ARM: OMAP2+: Add command line parameter > for debugSS module control > > Hi, > > * hvaibhav@ti.com <hvaibhav@ti.com> [130304 03:40]: > > From: Vaibhav Hiremath <hvaibhav@ti.com> > > > > Currently there is no clean mechanism to control debugSS module and > > you have to always keep clocks enabled, either, > > > > - By enabling it during boot as part of clk_init function. > > Or > > - By having HWMOD_INIT_NO_IDLE flag in hwmod data. > > > > Based on the discussion, > > http://www.mail-archive.com/linux-omap@vger.kernel.org/msg81771.html > > > > This patch introduces new kernel parameter "omap_debugss_en", > > which will allow user to control debugSS module enable/disable > > part during boot-time. > > I suggest you just make this part into a standard DT only > device driver. That way the command line parsing and clock > enabling can happen the normal way. > That’s good idea, as we are moving towards DT only boot support. Also, are you suggesting to have both command-line param and DT Property for this? > Is there any reason why this could not be a loadable module? > Because we want to keep it enabled before late_initcall. As part of late_initcall Clock/hwmod framework will start disabling unused modules, which will impact the Debugs as well. Consider the case where this debugss is loaded as a module, the user Will loose the JTAG connection until the module is loaded; and once module is Loaded, he has to again re-connect to the debugss. With only DT option the code will look like below, is this what you also have in your mind - arch/arm/boot/dts/am33xx.dtsi: debugss: debugss@4b000000 { compatible = "ti,debugss"; ti,hwmods = "debugss"; reg = <0x4b000000 1000000>; status = "disabled"; /* User need to enable it if he need JTAG connectivity*/ }; arch/arm/mach-omap2/debugss.c: static int __init _omap2_debugss_enable(void) { struct device_node *np; np = of_find_matching_node(oh_name); if (!node || ! of_device_is_available()) { pr_err("debugss device is not found\n"); return -ENODEV; } ... hwmod lookup./setup/enable along with optional clock enable. ... } device_initcall(_omap2_debugss_enable); Thanks, Vaibhav
* Hiremath, Vaibhav <hvaibhav@ti.com> [130409 01:12]: > > From: Tony Lindgren [mailto:tony@atomide.com] > > > > I suggest you just make this part into a standard DT only > > device driver. That way the command line parsing and clock > > enabling can happen the normal way. > > > > That’s good idea, as we are moving towards DT only boot support. > Also, are you suggesting to have both command-line param and DT > Property for this? > > > > Is there any reason why this could not be a loadable module? > > > > Because we want to keep it enabled before late_initcall. As part of late_initcall > Clock/hwmod framework will start disabling unused modules, which will impact the > Debugs as well. Consider the case where this debugss is loaded as a module, the user > Will loose the JTAG connection until the module is loaded; and once module is > Loaded, he has to again re-connect to the debugss. It will get run before late_initcall if compiled in. Sounds like there are no issues also make it work as a loadable module as needed. > With only DT option the code will look like below, is this what you also have in your mind - > > arch/arm/boot/dts/am33xx.dtsi: > > debugss: debugss@4b000000 { > compatible = "ti,debugss"; > ti,hwmods = "debugss"; > reg = <0x4b000000 1000000>; > status = "disabled"; /* User need to enable it if he need JTAG connectivity*/ > }; > > > arch/arm/mach-omap2/debugss.c: > > static int __init _omap2_debugss_enable(void) > { > struct device_node *np; > > np = of_find_matching_node(oh_name); > if (!node || ! of_device_is_available()) { > pr_err("debugss device is not found\n"); > return -ENODEV; > } > ... > hwmod lookup./setup/enable along with optional clock enable. > ... > > } > device_initcall(_omap2_debugss_enable); It should be all standard device driver stuff. I'd make it just regular module_platform_driver and only initialize it earlier if compiled in. Regards, Tony
> -----Original Message----- > From: Tony Lindgren [mailto:tony@atomide.com] > Sent: Tuesday, April 09, 2013 10:05 PM > To: Hiremath, Vaibhav > Cc: linux-omap@vger.kernel.org; khilman@linaro.org; paul@pwsan.com; > Nayak, Rajendra; linux-arm-kernel@lists.infradead.org > Subject: Re: [RFC PATCH 3/3] ARM: OMAP2+: Add command line parameter > for debugSS module control > > * Hiremath, Vaibhav <hvaibhav@ti.com> [130409 01:12]: > > > From: Tony Lindgren [mailto:tony@atomide.com] > > > > > > I suggest you just make this part into a standard DT only > > > device driver. That way the command line parsing and clock > > > enabling can happen the normal way. > > > > > > > That’s good idea, as we are moving towards DT only boot support. > > Also, are you suggesting to have both command-line param and DT > > Property for this? > > > > > > > Is there any reason why this could not be a loadable module? > > > > > > > Because we want to keep it enabled before late_initcall. As part of > late_initcall > > Clock/hwmod framework will start disabling unused modules, which will > impact the > > Debugs as well. Consider the case where this debugss is loaded as a > module, the user > > Will loose the JTAG connection until the module is loaded; and once > module is > > Loaded, he has to again re-connect to the debugss. > > It will get run before late_initcall if compiled in. Sounds > like there are no issues also make it work as a loadable module > as needed. > Agreed on making it as a module. But do you think we should allow it as a loadable module (M) as well, as user will loose Connectivity to JTAG during boot. Another perspective here would be, user can insert the module and Get JTAG connectivity, which also seems ok to me. Can you also confirm on having command line argument? I think We can only live with DT based approach, right? Thanks, Vaibhav
* Hiremath, Vaibhav <hvaibhav@ti.com> [130409 22:16]: > Agreed on making it as a module. > > But do you think we should allow it as a loadable module (M) as well, > as user will loose Connectivity to JTAG during boot. > Another perspective here would be, user can insert the module and > Get JTAG connectivity, which also seems ok to me. Yes both ways should be doable. > Can you also confirm on having command line argument? I think > We can only live with DT based approach, right? Well I doubt that anybody wants to keep it permanently enabled because of the power consumption and blocking of PM states, so an additional cmdline might make sense. It would be nice to have it most of the time built-in but disable itself unless something debug is specified in the cmdline. Maybe the JTAG driver can detect when the cable is connected? Or maybe that would be only when there's clock coming over JTAG? Regards, Tony
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt index 6c72381..bf1c822 100644 --- a/Documentation/kernel-parameters.txt +++ b/Documentation/kernel-parameters.txt @@ -2051,6 +2051,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted. For example, to override I2C bus2: omap_mux=i2c2_scl.i2c2_scl=0x100,i2c2_sda.i2c2_sda=0x100 + omap_debugss_en [OMAP] Enable Debug Sub-System module required + for JTAG debugging. + oprofile.timer= [HW] Use timer interrupt instead of performance counters diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile index d1156cf..aaf5cc2 100644 --- a/arch/arm/mach-omap2/Makefile +++ b/arch/arm/mach-omap2/Makefile @@ -5,7 +5,7 @@ # Common support obj-y := id.o io.o control.o mux.o devices.o fb.o serial.o gpmc.o timer.o pm.o \ common.o gpio.o dma.o wd_timer.o display.o i2c.o hdq1w.o omap_hwmod.o \ - omap_device.o sram.o + omap_device.o sram.o debugss.o omap-2-3-common = irq.o hwmod-common = omap_hwmod.o \ diff --git a/arch/arm/mach-omap2/debugss.c b/arch/arm/mach-omap2/debugss.c new file mode 100644 index 0000000..b45bf2c --- /dev/null +++ b/arch/arm/mach-omap2/debugss.c @@ -0,0 +1,80 @@ +/* + * debugss.c: Debug Sus-System related code goes in here + * + * Copyright (C) {2013} Texas Instruments Incorporated - http://www.ti.com/ + * + * This file is automatically generated from the AM33XX hardware databases. + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation version 2. + * + * This program is distributed "as is" WITHOUT ANY WARRANTY of any + * kind, whether express or implied; without even the implied warranty + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <linux/init.h> +#include <linux/err.h> +#include <linux/clk.h> + +#include "omap_hwmod.h" + +static bool is_debugss_en; + +/** + * omap_debugss_en - Enable debugss clock/module based on user config + * + * During kernel bootup, omap2 hwmod framework will disable all the + * unused/unclaimed modules, which in turn also disables debugss module. + * This breaks any further debugging capability provided by HW. + * + * + * Introduce early param which allows user to enable clock/module - + * + * omap_debugss_en (For all OMAP2 architectures) + * + * Please note that, with this command-line param, module always remain + * enabled. + */ +static int __init omap_debugss_en(char *str) +{ + is_debugss_en = true; + return 0; +} +early_param("omap_debugss_en", omap_debugss_en); + +static int __init _omap2_debugss_enable(void) +{ + const char oh_name[10] = "debugss"; + struct omap_hwmod *oh; + int ret; + + if (is_debugss_en) { + struct omap_hwmod_opt_clk *oc; + int i; + + oh = omap_hwmod_lookup(oh_name); + if (!oh) { + pr_err("debugss device not found\n"); + return 0; + } + + /* Make sure that hwmod internal data structures are setup */ + ret = omap_hwmod_setup_one(oh_name); + if (ret) { + pr_err("failed to setup hwmod for %s\n", oh_name); + return 0; + } + /* Enable optional clocks */ + for (i = oh->opt_clks_cnt, oc = oh->opt_clks; i > 0; i--, oc++) { + if (oc->_clk) + clk_prepare_enable(oc->_clk); + } + /* Enable debugss clock/module */ + omap_hwmod_enable(oh); + } + + return 0; +} +device_initcall(_omap2_debugss_enable);