| 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 > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
DQo+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+IEZyb206IFRvbnkgTGluZGdyZW4gW21h aWx0bzp0b255QGF0b21pZGUuY29tXQ0KPiBTZW50OiBUdWVzZGF5LCBBcHJpbCAwOSwgMjAxMyAx MDowNSBQTQ0KPiBUbzogSGlyZW1hdGgsIFZhaWJoYXYNCj4gQ2M6IGxpbnV4LW9tYXBAdmdlci5r ZXJuZWwub3JnOyBraGlsbWFuQGxpbmFyby5vcmc7IHBhdWxAcHdzYW4uY29tOw0KPiBOYXlhaywg UmFqZW5kcmE7IGxpbnV4LWFybS1rZXJuZWxAbGlzdHMuaW5mcmFkZWFkLm9yZw0KPiBTdWJqZWN0 OiBSZTogW1JGQyBQQVRDSCAzLzNdIEFSTTogT01BUDIrOiBBZGQgY29tbWFuZCBsaW5lIHBhcmFt ZXRlcg0KPiBmb3IgZGVidWdTUyBtb2R1bGUgY29udHJvbA0KPiANCj4gKiBIaXJlbWF0aCwgVmFp YmhhdiA8aHZhaWJoYXZAdGkuY29tPiBbMTMwNDA5IDAxOjEyXToNCj4gPiA+IEZyb206IFRvbnkg TGluZGdyZW4gW21haWx0bzp0b255QGF0b21pZGUuY29tXQ0KPiA+ID4NCj4gPiA+IEkgc3VnZ2Vz dCB5b3UganVzdCBtYWtlIHRoaXMgcGFydCBpbnRvIGEgc3RhbmRhcmQgRFQgb25seQ0KPiA+ID4g ZGV2aWNlIGRyaXZlci4gVGhhdCB3YXkgdGhlIGNvbW1hbmQgbGluZSBwYXJzaW5nIGFuZCBjbG9j aw0KPiA+ID4gZW5hYmxpbmcgY2FuIGhhcHBlbiB0aGUgbm9ybWFsIHdheS4NCj4gPiA+DQo+ID4N Cj4gPiBUaGF04oCZcyBnb29kIGlkZWEsIGFzIHdlIGFyZSBtb3ZpbmcgdG93YXJkcyBEVCBvbmx5 IGJvb3Qgc3VwcG9ydC4NCj4gPiBBbHNvLCBhcmUgeW91IHN1Z2dlc3RpbmcgdG8gaGF2ZSBib3Ro IGNvbW1hbmQtbGluZSBwYXJhbSBhbmQgRFQNCj4gPiBQcm9wZXJ0eSBmb3IgdGhpcz8NCj4gPg0K PiA+DQo+ID4gPiBJcyB0aGVyZSBhbnkgcmVhc29uIHdoeSB0aGlzIGNvdWxkIG5vdCBiZSBhIGxv YWRhYmxlIG1vZHVsZT8NCj4gPiA+DQo+ID4NCj4gPiBCZWNhdXNlIHdlIHdhbnQgdG8ga2VlcCBp dCBlbmFibGVkIGJlZm9yZSBsYXRlX2luaXRjYWxsLiBBcyBwYXJ0IG9mDQo+IGxhdGVfaW5pdGNh bGwNCj4gPiBDbG9jay9od21vZCBmcmFtZXdvcmsgd2lsbCBzdGFydCBkaXNhYmxpbmcgdW51c2Vk IG1vZHVsZXMsIHdoaWNoIHdpbGwNCj4gaW1wYWN0IHRoZQ0KPiA+IERlYnVncyBhcyB3ZWxsLiBD b25zaWRlciB0aGUgY2FzZSB3aGVyZSB0aGlzIGRlYnVnc3MgaXMgbG9hZGVkIGFzIGENCj4gbW9k dWxlLCB0aGUgdXNlcg0KPiA+IFdpbGwgbG9vc2UgdGhlIEpUQUcgY29ubmVjdGlvbiB1bnRpbCB0 aGUgbW9kdWxlIGlzIGxvYWRlZDsgYW5kIG9uY2UNCj4gbW9kdWxlIGlzDQo+ID4gTG9hZGVkLCBo ZSBoYXMgdG8gYWdhaW4gcmUtY29ubmVjdCB0byB0aGUgZGVidWdzcy4NCj4gDQo+IEl0IHdpbGwg Z2V0IHJ1biBiZWZvcmUgbGF0ZV9pbml0Y2FsbCBpZiBjb21waWxlZCBpbi4gU291bmRzDQo+IGxp a2UgdGhlcmUgYXJlIG5vIGlzc3VlcyBhbHNvIG1ha2UgaXQgd29yayBhcyBhIGxvYWRhYmxlIG1v ZHVsZQ0KPiBhcyBuZWVkZWQuDQo+IA0KQWdyZWVkIG9uIG1ha2luZyBpdCBhcyBhIG1vZHVsZS4N Cg0KQnV0IGRvIHlvdSB0aGluayB3ZSBzaG91bGQgYWxsb3cgaXQgYXMgYSBsb2FkYWJsZSBtb2R1 bGUgKE0pIGFzIHdlbGwsIA0KYXMgdXNlciB3aWxsIGxvb3NlIENvbm5lY3Rpdml0eSB0byBKVEFH IGR1cmluZyBib290Lg0KQW5vdGhlciBwZXJzcGVjdGl2ZSBoZXJlIHdvdWxkIGJlLCB1c2VyIGNh biBpbnNlcnQgdGhlIG1vZHVsZSBhbmQNCkdldCBKVEFHIGNvbm5lY3Rpdml0eSwgd2hpY2ggYWxz byBzZWVtcyBvayB0byBtZS4NCg0KQ2FuIHlvdSBhbHNvIGNvbmZpcm0gb24gaGF2aW5nIGNvbW1h bmQgbGluZSBhcmd1bWVudD8gSSB0aGluaw0KV2UgY2FuIG9ubHkgbGl2ZSB3aXRoIERUIGJhc2Vk IGFwcHJvYWNoLCByaWdodD8NCg0KVGhhbmtzLA0KVmFpYmhhdiANCg0K -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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);