Re: [PATCH] staging: tidspbridge: enable watchdog by default
On Tue, Feb 14, 2012 at 08:56:11PM -0600, Ramirez Luna, Omar wrote: On Tue, Feb 14, 2012 at 10:23 AM, Felipe Contreras felipe.contre...@gmail.com wrote: When that case is applicable, we should first modify the loader code or prepare the baseimages to be common so we can get rid of specific loaders and just dump them into memory. I'd say the less workarounds, the better. If there are ever more base images compatible with the dsp, I would say that unifying them into a common format to be dumped in memory isn't a workaround, and in that process we can get rid of the custom loader code. Yes! please! and use Ohad's rproc thingy. What would be the steps to unify that common format? I guess we will depend on TI for that... Do we? vmjl WDT could be detected by prepending common symbols into the baseimages or just making the new images to treat all peripherals as resources, that is, the new baseimg should actually request the wdt3 as any other clock. I see, but if wdt3 is requested as another clock, the Linux ARM side would still need to know how to threat the WDT. Tidspbridge does know how to treat other clock request from dsp (gpt, mcbsp), it would be a matter of adding the logic in the arm side for any dsp image that is able to do it, however this doesn't apply to the current (latest) base image since it assumes the wdt3 is always controlled by tidspbridge. Regards, Omar -- 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: [PATCH] staging: tidspbridge: enable watchdog by default
2012/2/15 Víctor M. Jáquez L. vjaq...@igalia.com: On Tue, Feb 14, 2012 at 08:56:11PM -0600, Ramirez Luna, Omar wrote: On Tue, Feb 14, 2012 at 10:23 AM, Felipe Contreras felipe.contre...@gmail.com wrote: When that case is applicable, we should first modify the loader code or prepare the baseimages to be common so we can get rid of specific loaders and just dump them into memory. I'd say the less workarounds, the better. If there are ever more base images compatible with the dsp, I would say that unifying them into a common format to be dumped in memory isn't a workaround, and in that process we can get rid of the custom loader code. Yes! please! and use Ohad's rproc thingy. I thought rproc is tied to elf for now. What would be the steps to unify that common format? I guess we will depend on TI for that... Do we? But this common format would be specific for tidspbridge, I don't think it makes sense for these images to have that constraint. Certainly rproc doesn't have it, and that one is not on staging. In any case, the proposed patch looks good. We can deal about these futuristic situations later on. Cheers. -- Felipe Contreras -- 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] staging: tidspbridge: enable watchdog by default
On Wed, Feb 15, 2012 at 01:41:19PM +0200, Felipe Contreras wrote: 2012/2/15 Víctor M. Jáquez L. vjaq...@igalia.com: On Tue, Feb 14, 2012 at 08:56:11PM -0600, Ramirez Luna, Omar wrote: On Tue, Feb 14, 2012 at 10:23 AM, Felipe Contreras felipe.contre...@gmail.com wrote: When that case is applicable, we should first modify the loader code or prepare the baseimages to be common so we can get rid of specific loaders and just dump them into memory. I'd say the less workarounds, the better. If there are ever more base images compatible with the dsp, I would say that unifying them into a common format to be dumped in memory isn't a workaround, and in that process we can get rid of the custom loader code. Yes! please! and use Ohad's rproc thingy. I thought rproc is tied to elf for now. It is. What would be the steps to unify that common format? I guess we will depend on TI for that... Do we? But this common format would be specific for tidspbridge, I don't think it makes sense for these images to have that constraint. Certainly rproc doesn't have it, and that one is not on staging. In any case, the proposed patch looks good. We can deal about these futuristic situations later on. I do agree. vmjl -- 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] staging: tidspbridge: enable watchdog by default
2012/2/15 Víctor M. Jáquez L. vjaq...@igalia.com: On Tue, Feb 14, 2012 at 08:56:11PM -0600, Ramirez Luna, Omar wrote: On Tue, Feb 14, 2012 at 10:23 AM, Felipe Contreras felipe.contre...@gmail.com wrote: When that case is applicable, we should first modify the loader code or prepare the baseimages to be common so we can get rid of specific loaders and just dump them into memory. I'd say the less workarounds, the better. If there are ever more base images compatible with the dsp, I would say that unifying them into a common format to be dumped in memory isn't a workaround, and in that process we can get rid of the custom loader code. Yes! please! and use Ohad's rproc thingy. Yes it would be nice, in theory we could replace some parts with remoteproc maybe not the loader yet, not sure how much rpmsg will fit. What would be the steps to unify that common format? I guess we will depend on TI for that... Do we? I don't know the stance of TI on this. But basically we have a loader in kernel space that prepares the baseimage to be dumped in memory, that can be done in user space or better to distribute a baseimage that just needs to be dumped in memory, however there might be some constraints that need to be solved like the tight coupling between the driver and the loader, the current format of the base image, the size and how it expands to fit in the 6MB of shm, and so on, not to mention the ability to load dynamic code. I guess it would be better to dive in the code and see. Regards, Omar -- 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] staging: tidspbridge: enable watchdog by default
On Tue, Feb 14, 2012 at 3:06 AM, Ramirez Luna, Omar omar.rami...@ti.com wrote: On Feb 11, 2012 3:03 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Sat, Feb 11, 2012 at 9:19 PM, Ramirez Luna, Omar omar.rami...@ti.com wrote: On Feb 10, 2012 12:44 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Fri, Feb 10, 2012 at 10:35 PM, Dan Carpenter dan.carpen...@oracle.com wrote: On Fri, Feb 10, 2012 at 09:42:32PM +0200, Felipe Contreras wrote: On Fri, Feb 10, 2012 at 8:00 PM, Dan Carpenter dan.carpen...@oracle.com wrote: On Fri, Feb 10, 2012 at 01:30:48AM +0200, Felipe Contreras wrote: It's not an oops, it's a warning, and again, it depends on the firmware being used. We don't have control over that, and we have no way to detect if this feature is there. It's up to the user. Perhaps just remove the warning message and handle the condition instead of printing a stack dump? The user should be triggering stack dumps. What on earth? The warning doesn't come from the driver. I'm not sure I understand. Are you saying that because it comes from the arch/ directory, it can't be fixed? I have good news for you my friend. :) It's all open source! \o/ The fact that you _can_ remove the warning doesn't mean you should. To me it sounds like a proper warning. Anyway, I saw in another email that Omar is working on a fix so probably we can just wait for his patch, yes? He only proposed a solution, I doubt he is working on. And to me, that sounded like a hack rather than a proper fix. I'm out on travel but will be able to look at it on Monday. Well, I think it is the right way, you look on the firmware if it has WDT you use it if it doesn't then you don't. Rather than guessing if you have the feature. It would be like reading a config option in the firmware. Yeah, but it's not really firmware, it's an operating system image. I can be running Linux there, and I might have implemented WDT. How is that code going to find that out? Tidspbridge loader doesn't support that use case, baseimage and let's say a dsp-linuximg won't have the same layout, hence it won't be able to parse and load the latter. When that case is applicable, we should first modify the loader code or prepare the baseimages to be common so we can get rid of specific loaders and just dump them into memory. I'd say the less workarounds, the better. WDT could be detected by prepending common symbols into the baseimages or just making the new images to treat all peripherals as resources, that is, the new baseimg should actually request the wdt3 as any other clock. I see, but if wdt3 is requested as another clock, the Linux ARM side would still need to know how to threat the WDT. Cheers. -- Felipe Contreras -- 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] staging: tidspbridge: enable watchdog by default
On Tue, Feb 14, 2012 at 10:23 AM, Felipe Contreras felipe.contre...@gmail.com wrote: When that case is applicable, we should first modify the loader code or prepare the baseimages to be common so we can get rid of specific loaders and just dump them into memory. I'd say the less workarounds, the better. If there are ever more base images compatible with the dsp, I would say that unifying them into a common format to be dumped in memory isn't a workaround, and in that process we can get rid of the custom loader code. WDT could be detected by prepending common symbols into the baseimages or just making the new images to treat all peripherals as resources, that is, the new baseimg should actually request the wdt3 as any other clock. I see, but if wdt3 is requested as another clock, the Linux ARM side would still need to know how to threat the WDT. Tidspbridge does know how to treat other clock request from dsp (gpt, mcbsp), it would be a matter of adding the logic in the arm side for any dsp image that is able to do it, however this doesn't apply to the current (latest) base image since it assumes the wdt3 is always controlled by tidspbridge. Regards, Omar -- 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] staging: tidspbridge: enable watchdog by default
On Feb 11, 2012 3:03 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Sat, Feb 11, 2012 at 9:19 PM, Ramirez Luna, Omar omar.rami...@ti.com wrote: On Feb 10, 2012 12:44 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Fri, Feb 10, 2012 at 10:35 PM, Dan Carpenter dan.carpen...@oracle.com wrote: On Fri, Feb 10, 2012 at 09:42:32PM +0200, Felipe Contreras wrote: On Fri, Feb 10, 2012 at 8:00 PM, Dan Carpenter dan.carpen...@oracle.com wrote: On Fri, Feb 10, 2012 at 01:30:48AM +0200, Felipe Contreras wrote: It's not an oops, it's a warning, and again, it depends on the firmware being used. We don't have control over that, and we have no way to detect if this feature is there. It's up to the user. Perhaps just remove the warning message and handle the condition instead of printing a stack dump? The user should be triggering stack dumps. What on earth? The warning doesn't come from the driver. I'm not sure I understand. Are you saying that because it comes from the arch/ directory, it can't be fixed? I have good news for you my friend. :) It's all open source! \o/ The fact that you _can_ remove the warning doesn't mean you should. To me it sounds like a proper warning. Anyway, I saw in another email that Omar is working on a fix so probably we can just wait for his patch, yes? He only proposed a solution, I doubt he is working on. And to me, that sounded like a hack rather than a proper fix. I'm out on travel but will be able to look at it on Monday. Well, I think it is the right way, you look on the firmware if it has WDT you use it if it doesn't then you don't. Rather than guessing if you have the feature. It would be like reading a config option in the firmware. Yeah, but it's not really firmware, it's an operating system image. I can be running Linux there, and I might have implemented WDT. How is that code going to find that out? Tidspbridge loader doesn't support that use case, baseimage and let's say a dsp-linuximg won't have the same layout, hence it won't be able to parse and load the latter. When that case is applicable, we should first modify the loader code or prepare the baseimages to be common so we can get rid of specific loaders and just dump them into memory. WDT could be detected by prepending common symbols into the baseimages or just making the new images to treat all peripherals as resources, that is, the new baseimg should actually request the wdt3 as any other clock. Regards, Omar -- 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] staging: tidspbridge: enable watchdog by default
On Thu, Feb 9, 2012 at 9:18 PM, Greg KH gre...@linuxfoundation.org wrote: On Thu, Feb 09, 2012 at 04:45:00PM -0800, Ramirez Luna, Omar wrote: Hi, On Thu, Feb 9, 2012 at 3:30 PM, Felipe Contreras felipe.contre...@gmail.com wrote: Again, I'm totally confused as to _WHY_ this needs to be y. What is causing this oops without it? If an oops is happening, then shouldn't this be a strict dependancy? Why allow it to be disabled at all if it can break your box if you don't enable it? It's not an oops, it's a warning, and again, it depends on the firmware being used. We don't have control over that, and we have no way to detect if this feature is there. It's up to the user. I have been thinking more into it, how about looking for a WDT symbol inside the baseimage to decide whether to turn ON/OFF WDT3, this would mean that the code is always compiled in, but the decision to turn it on/off is made at runtime. I totally don't understand, why not just silence the warning properly then? I fail to understand why this warning happens, why it depends on the firmware, and why you can't detect it at runtime to not do it. And how it all ties into a kconfig option... Just FTR, the warning comes from an interconnect driver that reports errors. This specific warning occurs because the dsp tries to access wdt3 registers without a clock, hence turning ON the wdt3 (or default y for wdt3 Kconfig) will make the warning to disappear when first loading a base image into the dsp, because now there will be a clock for the registers. It depends on the firmware because the accesses are made from the dsp itself. Regards, Omar -- 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] staging: tidspbridge: enable watchdog by default
On Sat, Feb 11, 2012 at 9:19 PM, Ramirez Luna, Omar omar.rami...@ti.com wrote: On Feb 10, 2012 12:44 PM, Felipe Contreras felipe.contre...@gmail.com wrote: On Fri, Feb 10, 2012 at 10:35 PM, Dan Carpenter dan.carpen...@oracle.com wrote: On Fri, Feb 10, 2012 at 09:42:32PM +0200, Felipe Contreras wrote: On Fri, Feb 10, 2012 at 8:00 PM, Dan Carpenter dan.carpen...@oracle.com wrote: On Fri, Feb 10, 2012 at 01:30:48AM +0200, Felipe Contreras wrote: It's not an oops, it's a warning, and again, it depends on the firmware being used. We don't have control over that, and we have no way to detect if this feature is there. It's up to the user. Perhaps just remove the warning message and handle the condition instead of printing a stack dump? The user should be triggering stack dumps. What on earth? The warning doesn't come from the driver. I'm not sure I understand. Are you saying that because it comes from the arch/ directory, it can't be fixed? I have good news for you my friend. :) It's all open source! \o/ The fact that you _can_ remove the warning doesn't mean you should. To me it sounds like a proper warning. Anyway, I saw in another email that Omar is working on a fix so probably we can just wait for his patch, yes? He only proposed a solution, I doubt he is working on. And to me, that sounded like a hack rather than a proper fix. I'm out on travel but will be able to look at it on Monday. Well, I think it is the right way, you look on the firmware if it has WDT you use it if it doesn't then you don't. Rather than guessing if you have the feature. It would be like reading a config option in the firmware. Yeah, but it's not really firmware, it's an operating system image. I can be running Linux there, and I might have implemented WDT. How is that code going to find that out? -- Felipe Contreras -- 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] staging: tidspbridge: enable watchdog by default
On Thu, 9 Feb 2012 21:18:49 -0800 Greg KH gre...@linuxfoundation.org wrote: On Thu, Feb 09, 2012 at 04:45:00PM -0800, Ramirez Luna, Omar wrote: Hi, On Thu, Feb 9, 2012 at 3:30 PM, Felipe Contreras felipe.contre...@gmail.com wrote: Again, I'm totally confused as to _WHY_ this needs to be y. What is causing this oops without it? If an oops is happening, then shouldn't this be a strict dependancy? Why allow it to be disabled at all if it can break your box if you don't enable it? It's not an oops, it's a warning, and again, it depends on the firmware being used. We don't have control over that, and we have no way to detect if this feature is there. It's up to the user. I have been thinking more into it, how about looking for a WDT symbol inside the baseimage to decide whether to turn ON/OFF WDT3, this would mean that the code is always compiled in, but the decision to turn it on/off is made at runtime. I totally don't understand, why not just silence the warning properly then? I fail to understand why this warning happens, why it depends on the firmware, and why you can't detect it at runtime to not do it. And how it all ties into a kconfig option... confused, greg k-h so there are _two_ issues that need to be fixed here: 1) the warning fix. 2) the whole default y thing that Mr. Torvalds is talking about. (the first fix(in my mind)would need to go first before anything else) Now reading through, greg had mentioned something about dependency, so if this is just dependency then add the proper Kconfig option, but if this is more than just a _dependency_ then somebody(who knows this code)is going to have to supply the proper fix for the warning(my C skills only take me so far!) below is my go at sending a _dependency_ fix for this, but could be totally wrong.. From 5c7ad6c00d051d574007cdbecdf14bf3d0cb Mon Sep 17 00:00:00 2001 From: Justin P. Mattock justinmatt...@gmail.com Date: Fri, 10 Feb 2012 07:19:45 -0800 Subject: [PATCH] Add dependency TIDSBRIDGE_WDT3 to TIDSBRIDGE. This would add the missing _dependency_ to tidsbridge to prevent a warning from happening. Note: my Kconfig skills are not the greatest so the below may or may not work. I can't test this because I dont have the hardware. Signed-off-by: Justin P. Mattock justinmatt...@gmail.com --- drivers/staging/tidspbridge/Kconfig |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/staging/tidspbridge/Kconfig b/drivers/staging/tidspbridge/Kconfig index 21a559e..7b385e0 100644 --- a/drivers/staging/tidspbridge/Kconfig +++ b/drivers/staging/tidspbridge/Kconfig @@ -6,6 +6,7 @@ menuconfig TIDSPBRIDGE tristate DSP Bridge driver depends on ARCH_OMAP3 select OMAP_MBOX_FWK + select TIDSPBRIDGE_WDT3 help DSP/BIOS Bridge is designed for platforms that contain a GPP and one or more attached DSPs. The GPP is considered the master or -- 1.7.9 -- Justin P. Mattock justinmatt...@gmail.com -- 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] staging: tidspbridge: enable watchdog by default
On Fri, Feb 10, 2012 at 5:35 PM, Justin P. Mattock justinmatt...@gmail.com wrote: On Thu, 9 Feb 2012 21:18:49 -0800 Greg KH gre...@linuxfoundation.org wrote: On Thu, Feb 09, 2012 at 04:45:00PM -0800, Ramirez Luna, Omar wrote: Hi, On Thu, Feb 9, 2012 at 3:30 PM, Felipe Contreras felipe.contre...@gmail.com wrote: Again, I'm totally confused as to _WHY_ this needs to be y. What is causing this oops without it? If an oops is happening, then shouldn't this be a strict dependancy? Why allow it to be disabled at all if it can break your box if you don't enable it? It's not an oops, it's a warning, and again, it depends on the firmware being used. We don't have control over that, and we have no way to detect if this feature is there. It's up to the user. I have been thinking more into it, how about looking for a WDT symbol inside the baseimage to decide whether to turn ON/OFF WDT3, this would mean that the code is always compiled in, but the decision to turn it on/off is made at runtime. I totally don't understand, why not just silence the warning properly then? I fail to understand why this warning happens, why it depends on the firmware, and why you can't detect it at runtime to not do it. And how it all ties into a kconfig option... confused, greg k-h so there are _two_ issues that need to be fixed here: 1) the warning fix. 2) the whole default y thing that Mr. Torvalds is talking about. (the first fix(in my mind)would need to go first before anything else) Now reading through, greg had mentioned something about dependency, so if this is just dependency then add the proper Kconfig option, but if this is more than just a _dependency_ then somebody(who knows this code)is going to have to supply the proper fix for the warning(my C skills only take me so far!) below is my go at sending a _dependency_ fix for this, but could be totally wrong.. From 5c7ad6c00d051d574007cdbecdf14bf3d0cb Mon Sep 17 00:00:00 2001 From: Justin P. Mattock justinmatt...@gmail.com Date: Fri, 10 Feb 2012 07:19:45 -0800 Subject: [PATCH] Add dependency TIDSBRIDGE_WDT3 to TIDSBRIDGE. This would add the missing _dependency_ to tidsbridge to prevent a warning from happening. Note: my Kconfig skills are not the greatest so the below may or may not work. I can't test this because I dont have the hardware. Your patch *always* turns on TIDSPBRIDGE_WDT3, which is not what we want. Depending on the firmware, some people might want it off. Basically, right now on the typical firmware, people have to either manually turn TIDSPBRIDGE_WDT3 on, or they will see the warning. Cheers. -- Felipe Contreras -- 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] staging: tidspbridge: enable watchdog by default
On Fri, 10 Feb 2012 18:05:59 +0200 Felipe Contreras felipe.contre...@gmail.com wrote: On Fri, Feb 10, 2012 at 5:35 PM, Justin P. Mattock justinmatt...@gmail.com wrote: On Thu, 9 Feb 2012 21:18:49 -0800 Greg KH gre...@linuxfoundation.org wrote: On Thu, Feb 09, 2012 at 04:45:00PM -0800, Ramirez Luna, Omar wrote: Hi, On Thu, Feb 9, 2012 at 3:30 PM, Felipe Contreras felipe.contre...@gmail.com wrote: Again, I'm totally confused as to _WHY_ this needs to be y. What is causing this oops without it? If an oops is happening, then shouldn't this be a strict dependancy? Why allow it to be disabled at all if it can break your box if you don't enable it? It's not an oops, it's a warning, and again, it depends on the firmware being used. We don't have control over that, and we have no way to detect if this feature is there. It's up to the user. I have been thinking more into it, how about looking for a WDT symbol inside the baseimage to decide whether to turn ON/OFF WDT3, this would mean that the code is always compiled in, but the decision to turn it on/off is made at runtime. I totally don't understand, why not just silence the warning properly then? I fail to understand why this warning happens, why it depends on the firmware, and why you can't detect it at runtime to not do it. And how it all ties into a kconfig option... confused, greg k-h so there are _two_ issues that need to be fixed here: 1) the warning fix. 2) the whole default y thing that Mr. Torvalds is talking about. (the first fix(in my mind)would need to go first before anything else) Now reading through, greg had mentioned something about dependency, so if this is just dependency then add the proper Kconfig option, but if this is more than just a _dependency_ then somebody(who knows this code)is going to have to supply the proper fix for the warning(my C skills only take me so far!) below is my go at sending a _dependency_ fix for this, but could be totally wrong.. From 5c7ad6c00d051d574007cdbecdf14bf3d0cb Mon Sep 17 00:00:00 2001 From: Justin P. Mattock justinmatt...@gmail.com Date: Fri, 10 Feb 2012 07:19:45 -0800 Subject: [PATCH] Add dependency TIDSBRIDGE_WDT3 to TIDSBRIDGE. This would add the missing _dependency_ to tidsbridge to prevent a warning from happening. Note: my Kconfig skills are not the greatest so the below may or may not work. I can't test this because I dont have the hardware. Your patch *always* turns on TIDSPBRIDGE_WDT3, which is not what we want. Depending on the firmware, some people might want it off. Basically, right now on the typical firmware, people have to either manually turn TIDSPBRIDGE_WDT3 on, or they will see the warning. Cheers. -- Felipe Contreras ahh!! I see.. then maybe it needs be something like what omar had suggested, or maybe even some dmi_system_id(but the kernel has to many of those). -- Justin P. Mattock justinmatt...@gmail.com -- 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] staging: tidspbridge: enable watchdog by default
On Fri, Feb 10, 2012 at 7:18 AM, Greg KH gre...@linuxfoundation.org wrote: On Thu, Feb 09, 2012 at 04:45:00PM -0800, Ramirez Luna, Omar wrote: Hi, On Thu, Feb 9, 2012 at 3:30 PM, Felipe Contreras felipe.contre...@gmail.com wrote: Again, I'm totally confused as to _WHY_ this needs to be y. What is causing this oops without it? If an oops is happening, then shouldn't this be a strict dependancy? Why allow it to be disabled at all if it can break your box if you don't enable it? It's not an oops, it's a warning, and again, it depends on the firmware being used. We don't have control over that, and we have no way to detect if this feature is there. It's up to the user. I have been thinking more into it, how about looking for a WDT symbol inside the baseimage to decide whether to turn ON/OFF WDT3, this would mean that the code is always compiled in, but the decision to turn it on/off is made at runtime. I totally don't understand, why not just silence the warning properly then? The warning doesn't come from the driver, and AFAIK it's a valid warning. Now, I haven't explored the code, so I don't know what's going on, I just know Omar said this fixes the warning, and that there's no easy way around it. I fail to understand why this warning happens, I don't know either, I just know enabling TIDSPBRIDGE_WDT3 fixes it. why it depends on the firmware, Because it just does. Does your firmware have WDT? Yes = TIDSPBRIDGE_WDT3 on, No = TIDSPBRIDGE_WDT3 off. What is so complicated about this? and why you can't detect it at runtime to not do it. The DSP firmwmare is an operating system. You even run Linux on it. We just don't know if has WDT support or not. Omar is familiar with the code regarding the WDT, and he said it was not easy to detect if enabling WDT failed or not. And how it all ties into a kconfig option... Some people want it, some people don't. -- Felipe Contreras -- 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] staging: tidspbridge: enable watchdog by default
On Fri, Feb 10, 2012 at 06:05:59PM +0200, Felipe Contreras wrote: From 5c7ad6c00d051d574007cdbecdf14bf3d0cb Mon Sep 17 00:00:00 2001 From: Justin P. Mattock justinmatt...@gmail.com Date: Fri, 10 Feb 2012 07:19:45 -0800 Subject: [PATCH] Add dependency TIDSBRIDGE_WDT3 to TIDSBRIDGE. This would add the missing _dependency_ to tidsbridge to prevent a warning from happening. Note: my Kconfig skills are not the greatest so the below may or may not work. I can't test this because I dont have the hardware. Your patch *always* turns on TIDSPBRIDGE_WDT3, which is not what we want. Depending on the firmware, some people might want it off. What firmware? Why not document this properly somewhere in the help entries? Why not detect this automatically in the kernel based on the firmware version? Basically, right now on the typical firmware, people have to either manually turn TIDSPBRIDGE_WDT3 on, or they will see the warning. So, for the typical firmware, you do want this on, so the patch makes sense. How about the code be fixed so that it doesn't generate this type of warning when using the typical firmware, instead of having to rely on confusing Kconfig entries. Still confused, greg k-h -- 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] staging: tidspbridge: enable watchdog by default
On Fri, Feb 10, 2012 at 6:16 PM, Greg KH gre...@linuxfoundation.org wrote: On Fri, Feb 10, 2012 at 06:05:59PM +0200, Felipe Contreras wrote: From 5c7ad6c00d051d574007cdbecdf14bf3d0cb Mon Sep 17 00:00:00 2001 From: Justin P. Mattock justinmatt...@gmail.com Date: Fri, 10 Feb 2012 07:19:45 -0800 Subject: [PATCH] Add dependency TIDSBRIDGE_WDT3 to TIDSBRIDGE. This would add the missing _dependency_ to tidsbridge to prevent a warning from happening. Note: my Kconfig skills are not the greatest so the below may or may not work. I can't test this because I dont have the hardware. Your patch *always* turns on TIDSPBRIDGE_WDT3, which is not what we want. Depending on the firmware, some people might want it off. What firmware? What do you think is the purpose of tidspbridge? Why not document this properly somewhere in the help entries? It is documented (kinda). If you are using a non-standard baseimage, you should know what is the watchdog timer, and whether or not your baseimage supports it. Fortunately, the vast majority of users don't need to care about it. They can just use the default. Why not detect this automatically in the kernel based on the firmware version? There is no firmware version. The firmware, or rather baseimage, is an operating system. You can't detect if Linux has USB support based on the Linux version, can you? Basically, right now on the typical firmware, people have to either manually turn TIDSPBRIDGE_WDT3 on, or they will see the warning. So, for the typical firmware, you do want this on, so the patch makes sense. Yes. How about the code be fixed so that it doesn't generate this type of warning when using the typical firmware, instead of having to rely on confusing Kconfig entries. Patches welcome. I for one don't know a way, and I'd rather concentrate on one of the myriad other tasks that need to be done for this driver. I just thought it would be nice to make it easier for most users to avoid this warning. Cheers. -- Felipe Contreras -- 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] staging: tidspbridge: enable watchdog by default
On Fri, Feb 10, 2012 at 06:16:19PM +0200, Felipe Contreras wrote: On Fri, Feb 10, 2012 at 7:18 AM, Greg KH gre...@linuxfoundation.org wrote: On Thu, Feb 09, 2012 at 04:45:00PM -0800, Ramirez Luna, Omar wrote: Hi, On Thu, Feb 9, 2012 at 3:30 PM, Felipe Contreras felipe.contre...@gmail.com wrote: Again, I'm totally confused as to _WHY_ this needs to be y. What is causing this oops without it? If an oops is happening, then shouldn't this be a strict dependancy? Why allow it to be disabled at all if it can break your box if you don't enable it? It's not an oops, it's a warning, and again, it depends on the firmware being used. We don't have control over that, and we have no way to detect if this feature is there. It's up to the user. I have been thinking more into it, how about looking for a WDT symbol inside the baseimage to decide whether to turn ON/OFF WDT3, this would mean that the code is always compiled in, but the decision to turn it on/off is made at runtime. I totally don't understand, why not just silence the warning properly then? The warning doesn't come from the driver, and AFAIK it's a valid warning. Now, I haven't explored the code, so I don't know what's going on, I just know Omar said this fixes the warning, and that there's no easy way around it. I fail to understand why this warning happens, I don't know either, I just know enabling TIDSPBRIDGE_WDT3 fixes it. Ok, I suggest someone get to the root cause of this before we change anything, although some better Kconfig text would be nice to have to document the existing issues. thanks, greg k-h -- 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] staging: tidspbridge: enable watchdog by default
On Fri, Feb 10, 2012 at 01:30:48AM +0200, Felipe Contreras wrote: It's not an oops, it's a warning, and again, it depends on the firmware being used. We don't have control over that, and we have no way to detect if this feature is there. It's up to the user. Perhaps just remove the warning message and handle the condition instead of printing a stack dump? The user should be triggering stack dumps. What on earth? regards, dan carpenter signature.asc Description: Digital signature
Re: [PATCH] staging: tidspbridge: enable watchdog by default
On Fri, Feb 10, 2012 at 8:00 PM, Dan Carpenter dan.carpen...@oracle.com wrote: On Fri, Feb 10, 2012 at 01:30:48AM +0200, Felipe Contreras wrote: It's not an oops, it's a warning, and again, it depends on the firmware being used. We don't have control over that, and we have no way to detect if this feature is there. It's up to the user. Perhaps just remove the warning message and handle the condition instead of printing a stack dump? The user should be triggering stack dumps. What on earth? The warning doesn't come from the driver. -- Felipe Contreras -- 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] staging: tidspbridge: enable watchdog by default
On Fri, Feb 10, 2012 at 09:42:32PM +0200, Felipe Contreras wrote: On Fri, Feb 10, 2012 at 8:00 PM, Dan Carpenter dan.carpen...@oracle.com wrote: On Fri, Feb 10, 2012 at 01:30:48AM +0200, Felipe Contreras wrote: It's not an oops, it's a warning, and again, it depends on the firmware being used. We don't have control over that, and we have no way to detect if this feature is there. It's up to the user. Perhaps just remove the warning message and handle the condition instead of printing a stack dump? The user should be triggering stack dumps. What on earth? The warning doesn't come from the driver. I'm not sure I understand. Are you saying that because it comes from the arch/ directory, it can't be fixed? I have good news for you my friend. :) It's all open source! \o/ Anyway, I saw in another email that Omar is working on a fix so probably we can just wait for his patch, yes? regards, dan carpenter signature.asc Description: Digital signature
Re: [PATCH] staging: tidspbridge: enable watchdog by default
On Fri, Feb 10, 2012 at 10:35 PM, Dan Carpenter dan.carpen...@oracle.com wrote: On Fri, Feb 10, 2012 at 09:42:32PM +0200, Felipe Contreras wrote: On Fri, Feb 10, 2012 at 8:00 PM, Dan Carpenter dan.carpen...@oracle.com wrote: On Fri, Feb 10, 2012 at 01:30:48AM +0200, Felipe Contreras wrote: It's not an oops, it's a warning, and again, it depends on the firmware being used. We don't have control over that, and we have no way to detect if this feature is there. It's up to the user. Perhaps just remove the warning message and handle the condition instead of printing a stack dump? The user should be triggering stack dumps. What on earth? The warning doesn't come from the driver. I'm not sure I understand. Are you saying that because it comes from the arch/ directory, it can't be fixed? I have good news for you my friend. :) It's all open source! \o/ The fact that you _can_ remove the warning doesn't mean you should. To me it sounds like a proper warning. Anyway, I saw in another email that Omar is working on a fix so probably we can just wait for his patch, yes? He only proposed a solution, I doubt he is working on. And to me, that sounded like a hack rather than a proper fix. -- Felipe Contreras -- 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] staging: tidspbridge: enable watchdog by default
On Wed, Feb 01, 2012 at 09:26:16AM +0200, Felipe Contreras wrote: On Wed, Feb 1, 2012 at 5:44 AM, Greg KH g...@kroah.com wrote: On Wed, Feb 01, 2012 at 03:37:55AM +0200, Felipe Contreras wrote: On Wed, Feb 1, 2012 at 3:12 AM, Greg KH gre...@suse.de wrote: On Wed, Feb 01, 2012 at 03:01:32AM +0200, Felipe Contreras wrote: From: Felipe Contreras felipe.contre...@nokia.com The public images have it enabled, it's safer, and we get rid of this warning: WARNING: at arch/arm/mach-omap2/omap_l3_smx.c:162 omap3_l3_app_irq+0x114/0x15c() In-band Error seen by IVA_SS at address 0 Modules linked in: bridgedriver(C+) mailbox_mach mailbox ipv6 g_ether [c0012848] (unwind_backtrace+0x0/0xec) from [c002e760] (warn_slowpath_common+0x4c/0x64) [c002e760] (warn_slowpath_common+0x4c/0x64) from [c002e7f8] (warn_slowpath_fmt+0x2c/0x3c) [c002e7f8] (warn_slowpath_fmt+0x2c/0x3c) from [c00217c8] (omap3_l3_app_irq+0x114/0x15c) [c00217c8] (omap3_l3_app_irq+0x114/0x15c) from [c0059844] (handle_irq_event_percpu+0x28/0x174) [c0059844] (handle_irq_event_percpu+0x28/0x174) from [c00599b8] (handle_irq_event+0x28/0x38) [c00599b8] (handle_irq_event+0x28/0x38) from [c005b8d0] (handle_level_irq+0xb8/0xe0) [c005b8d0] (handle_level_irq+0xb8/0xe0) from [c0059598] (generic_handle_irq+0x28/0x30) [c0059598] (generic_handle_irq+0x28/0x30) from [c000e62c] (handle_IRQ+0x60/0x84) [c000e62c] (handle_IRQ+0x60/0x84) from [c000d334] (__irq_svc+0x34/0x80) [c000d334] (__irq_svc+0x34/0x80) from [c0015804] (v7_dma_inv_range+0x34/0x48) [c0015804] (v7_dma_inv_range+0x34/0x48) from [c00129ec] (dma_cache_maint_page+0x2c/0x34) [c00129ec] (dma_cache_maint_page+0x2c/0x34) from [c0012a34] (___dma_page_dev_to_cpu+0x24/0x68) [c0012a34] (___dma_page_dev_to_cpu+0x24/0x68) from [c0012b50] (dma_unmap_sg+0x40/0x64) [c0012b50] (dma_unmap_sg+0x40/0x64) from [c019fba4] (omap_hsmmc_post_req+0x3c/0x48) [c019fba4] (omap_hsmmc_post_req+0x3c/0x48) from [c01932f4] (mmc_post_req+0x18/0x1c) [c01932f4] (mmc_post_req+0x18/0x1c) from [c019535c] (mmc_start_req+0xd4/0xec) [c019535c] (mmc_start_req+0xd4/0xec) from [c019d330] (mmc_blk_issue_rw_rq+0x64/0x244) [c019d330] (mmc_blk_issue_rw_rq+0x64/0x244) from [c019d7bc] (mmc_blk_issue_rq+0x2ac/0x2cc) [c019d7bc] (mmc_blk_issue_rq+0x2ac/0x2cc) from [c019e1c0] (mmc_queue_thread+0x8c/0xf8) [c019e1c0] (mmc_queue_thread+0x8c/0xf8) from [c0044c10] (kthread+0x84/0x8c) [c0044c10] (kthread+0x84/0x8c) from [c000e694] (kernel_thread_exit+0x0/0x8) ---[ end trace 5dec1c8d7857375d ]--- Signed-off-by: Felipe Contreras felipe.contre...@nokia.com --- drivers/staging/tidspbridge/Kconfig | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/staging/tidspbridge/Kconfig b/drivers/staging/tidspbridge/Kconfig index 21a559e..614e974 100644 --- a/drivers/staging/tidspbridge/Kconfig +++ b/drivers/staging/tidspbridge/Kconfig @@ -61,6 +61,7 @@ config TIDSPBRIDGE_CACHE_LINE_CHECK config TIDSPBRIDGE_WDT3 bool Enable watchdog timer depends on TIDSPBRIDGE + default y Nothing is ever default y, sorry, I can't accept this. % git grep 'default y' -- 'drivers/*/Kconfig' | wc -l 493 That's nice, but proves nothing, no new entries should be y. If it should be y, then it should just not be an option and always enabled, right? Why not do that? What if I compiled a DSP baseimage that doesn't have this watchdog? I don't know, you tell me. And using that logic, we should start removing all configurations in the kernel that have 'default y'. If your machine needs it to boot, then no, do not. No. There's a reason they are *configurations*, and there's a reason they are 'default y'. Again, no NEW config options should be default y unless you really have a good reason. And I'll take that a step further and say, if you want this to be default y, then why even make it a config option at all and not just build it in whenever the dependancy is detected? So, care to just send a patch removing this config option instead? Or, if bad things happen if the option is turned off, then fix those, don't paper over it by trying to enable the option, that's not a real fix. greg k-h -- 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] staging: tidspbridge: enable watchdog by default
On Thu, Feb 9, 2012 at 7:35 PM, Greg KH gre...@linuxfoundation.org wrote: On Wed, Feb 01, 2012 at 09:26:16AM +0200, Felipe Contreras wrote: On Wed, Feb 1, 2012 at 5:44 AM, Greg KH g...@kroah.com wrote: On Wed, Feb 01, 2012 at 03:37:55AM +0200, Felipe Contreras wrote: On Wed, Feb 1, 2012 at 3:12 AM, Greg KH gre...@suse.de wrote: On Wed, Feb 01, 2012 at 03:01:32AM +0200, Felipe Contreras wrote: From: Felipe Contreras felipe.contre...@nokia.com The public images have it enabled, it's safer, and we get rid of this warning: WARNING: at arch/arm/mach-omap2/omap_l3_smx.c:162 omap3_l3_app_irq+0x114/0x15c() In-band Error seen by IVA_SS at address 0 Modules linked in: bridgedriver(C+) mailbox_mach mailbox ipv6 g_ether [c0012848] (unwind_backtrace+0x0/0xec) from [c002e760] (warn_slowpath_common+0x4c/0x64) [c002e760] (warn_slowpath_common+0x4c/0x64) from [c002e7f8] (warn_slowpath_fmt+0x2c/0x3c) [c002e7f8] (warn_slowpath_fmt+0x2c/0x3c) from [c00217c8] (omap3_l3_app_irq+0x114/0x15c) [c00217c8] (omap3_l3_app_irq+0x114/0x15c) from [c0059844] (handle_irq_event_percpu+0x28/0x174) [c0059844] (handle_irq_event_percpu+0x28/0x174) from [c00599b8] (handle_irq_event+0x28/0x38) [c00599b8] (handle_irq_event+0x28/0x38) from [c005b8d0] (handle_level_irq+0xb8/0xe0) [c005b8d0] (handle_level_irq+0xb8/0xe0) from [c0059598] (generic_handle_irq+0x28/0x30) [c0059598] (generic_handle_irq+0x28/0x30) from [c000e62c] (handle_IRQ+0x60/0x84) [c000e62c] (handle_IRQ+0x60/0x84) from [c000d334] (__irq_svc+0x34/0x80) [c000d334] (__irq_svc+0x34/0x80) from [c0015804] (v7_dma_inv_range+0x34/0x48) [c0015804] (v7_dma_inv_range+0x34/0x48) from [c00129ec] (dma_cache_maint_page+0x2c/0x34) [c00129ec] (dma_cache_maint_page+0x2c/0x34) from [c0012a34] (___dma_page_dev_to_cpu+0x24/0x68) [c0012a34] (___dma_page_dev_to_cpu+0x24/0x68) from [c0012b50] (dma_unmap_sg+0x40/0x64) [c0012b50] (dma_unmap_sg+0x40/0x64) from [c019fba4] (omap_hsmmc_post_req+0x3c/0x48) [c019fba4] (omap_hsmmc_post_req+0x3c/0x48) from [c01932f4] (mmc_post_req+0x18/0x1c) [c01932f4] (mmc_post_req+0x18/0x1c) from [c019535c] (mmc_start_req+0xd4/0xec) [c019535c] (mmc_start_req+0xd4/0xec) from [c019d330] (mmc_blk_issue_rw_rq+0x64/0x244) [c019d330] (mmc_blk_issue_rw_rq+0x64/0x244) from [c019d7bc] (mmc_blk_issue_rq+0x2ac/0x2cc) [c019d7bc] (mmc_blk_issue_rq+0x2ac/0x2cc) from [c019e1c0] (mmc_queue_thread+0x8c/0xf8) [c019e1c0] (mmc_queue_thread+0x8c/0xf8) from [c0044c10] (kthread+0x84/0x8c) [c0044c10] (kthread+0x84/0x8c) from [c000e694] (kernel_thread_exit+0x0/0x8) ---[ end trace 5dec1c8d7857375d ]--- Signed-off-by: Felipe Contreras felipe.contre...@nokia.com --- drivers/staging/tidspbridge/Kconfig | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/staging/tidspbridge/Kconfig b/drivers/staging/tidspbridge/Kconfig index 21a559e..614e974 100644 --- a/drivers/staging/tidspbridge/Kconfig +++ b/drivers/staging/tidspbridge/Kconfig @@ -61,6 +61,7 @@ config TIDSPBRIDGE_CACHE_LINE_CHECK config TIDSPBRIDGE_WDT3 bool Enable watchdog timer depends on TIDSPBRIDGE + default y Nothing is ever default y, sorry, I can't accept this. % git grep 'default y' -- 'drivers/*/Kconfig' | wc -l 493 That's nice, but proves nothing, no new entries should be y. If it should be y, then it should just not be an option and always enabled, right? Why not do that? What if I compiled a DSP baseimage that doesn't have this watchdog? I don't know, you tell me. Then you want this option disabled. And using that logic, we should start removing all configurations in the kernel that have 'default y'. If your machine needs it to boot, then no, do not. There's many configurations that are not needed for booting, yet they are 'default y'. No. There's a reason they are *configurations*, and there's a reason they are 'default y'. Again, no NEW config options should be default y unless you really have a good reason. I already gave you a good reason, and this came from Linus Torvalds. http://article.gmane.org/gmane.linux.kernel/995419 Notice how he agreed that setting defaults was a good idea. I can't point to the mails of other people that also agreed this was the direction to go. You keep saying no options should be 'default y', but you don't say where that policy comes from. And I'll take that a step further and say, if you want this to be default y, then why even make it a config option at all and not just build it in whenever the dependancy is detected? The dependency can't be detected. So, care to just send a patch removing this config option instead? No. That's not what anybody wants. Or, if bad things happen if the option is turned off, then fix those, don't paper over it by trying to enable the option, that's not
Re: [PATCH] staging: tidspbridge: enable watchdog by default
On Thu, Feb 9, 2012 at 8:41 PM, Felipe Contreras felipe.contre...@gmail.com wrote: I already gave you a good reason, and this came from Linus Torvalds. http://article.gmane.org/gmane.linux.kernel/995419 Notice how he agreed that setting defaults was a good idea. I can't point to the mails of other people that also agreed this was the direction to go. Er, I can, if you want. -- Felipe Contreras -- 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] staging: tidspbridge: enable watchdog by default
On Thu, Feb 09, 2012 at 08:41:55PM +0200, Felipe Contreras wrote: On Thu, Feb 9, 2012 at 7:35 PM, Greg KH gre...@linuxfoundation.org wrote: On Wed, Feb 01, 2012 at 09:26:16AM +0200, Felipe Contreras wrote: On Wed, Feb 1, 2012 at 5:44 AM, Greg KH g...@kroah.com wrote: On Wed, Feb 01, 2012 at 03:37:55AM +0200, Felipe Contreras wrote: On Wed, Feb 1, 2012 at 3:12 AM, Greg KH gre...@suse.de wrote: On Wed, Feb 01, 2012 at 03:01:32AM +0200, Felipe Contreras wrote: From: Felipe Contreras felipe.contre...@nokia.com The public images have it enabled, it's safer, and we get rid of this warning: WARNING: at arch/arm/mach-omap2/omap_l3_smx.c:162 omap3_l3_app_irq+0x114/0x15c() In-band Error seen by IVA_SS at address 0 Modules linked in: bridgedriver(C+) mailbox_mach mailbox ipv6 g_ether [c0012848] (unwind_backtrace+0x0/0xec) from [c002e760] (warn_slowpath_common+0x4c/0x64) [c002e760] (warn_slowpath_common+0x4c/0x64) from [c002e7f8] (warn_slowpath_fmt+0x2c/0x3c) [c002e7f8] (warn_slowpath_fmt+0x2c/0x3c) from [c00217c8] (omap3_l3_app_irq+0x114/0x15c) [c00217c8] (omap3_l3_app_irq+0x114/0x15c) from [c0059844] (handle_irq_event_percpu+0x28/0x174) [c0059844] (handle_irq_event_percpu+0x28/0x174) from [c00599b8] (handle_irq_event+0x28/0x38) [c00599b8] (handle_irq_event+0x28/0x38) from [c005b8d0] (handle_level_irq+0xb8/0xe0) [c005b8d0] (handle_level_irq+0xb8/0xe0) from [c0059598] (generic_handle_irq+0x28/0x30) [c0059598] (generic_handle_irq+0x28/0x30) from [c000e62c] (handle_IRQ+0x60/0x84) [c000e62c] (handle_IRQ+0x60/0x84) from [c000d334] (__irq_svc+0x34/0x80) [c000d334] (__irq_svc+0x34/0x80) from [c0015804] (v7_dma_inv_range+0x34/0x48) [c0015804] (v7_dma_inv_range+0x34/0x48) from [c00129ec] (dma_cache_maint_page+0x2c/0x34) [c00129ec] (dma_cache_maint_page+0x2c/0x34) from [c0012a34] (___dma_page_dev_to_cpu+0x24/0x68) [c0012a34] (___dma_page_dev_to_cpu+0x24/0x68) from [c0012b50] (dma_unmap_sg+0x40/0x64) [c0012b50] (dma_unmap_sg+0x40/0x64) from [c019fba4] (omap_hsmmc_post_req+0x3c/0x48) [c019fba4] (omap_hsmmc_post_req+0x3c/0x48) from [c01932f4] (mmc_post_req+0x18/0x1c) [c01932f4] (mmc_post_req+0x18/0x1c) from [c019535c] (mmc_start_req+0xd4/0xec) [c019535c] (mmc_start_req+0xd4/0xec) from [c019d330] (mmc_blk_issue_rw_rq+0x64/0x244) [c019d330] (mmc_blk_issue_rw_rq+0x64/0x244) from [c019d7bc] (mmc_blk_issue_rq+0x2ac/0x2cc) [c019d7bc] (mmc_blk_issue_rq+0x2ac/0x2cc) from [c019e1c0] (mmc_queue_thread+0x8c/0xf8) [c019e1c0] (mmc_queue_thread+0x8c/0xf8) from [c0044c10] (kthread+0x84/0x8c) [c0044c10] (kthread+0x84/0x8c) from [c000e694] (kernel_thread_exit+0x0/0x8) ---[ end trace 5dec1c8d7857375d ]--- Signed-off-by: Felipe Contreras felipe.contre...@nokia.com --- drivers/staging/tidspbridge/Kconfig | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/staging/tidspbridge/Kconfig b/drivers/staging/tidspbridge/Kconfig index 21a559e..614e974 100644 --- a/drivers/staging/tidspbridge/Kconfig +++ b/drivers/staging/tidspbridge/Kconfig @@ -61,6 +61,7 @@ config TIDSPBRIDGE_CACHE_LINE_CHECK config TIDSPBRIDGE_WDT3 bool Enable watchdog timer depends on TIDSPBRIDGE + default y Nothing is ever default y, sorry, I can't accept this. % git grep 'default y' -- 'drivers/*/Kconfig' | wc -l 493 That's nice, but proves nothing, no new entries should be y. If it should be y, then it should just not be an option and always enabled, right? Why not do that? What if I compiled a DSP baseimage that doesn't have this watchdog? I don't know, you tell me. Then you want this option disabled. And using that logic, we should start removing all configurations in the kernel that have 'default y'. If your machine needs it to boot, then no, do not. There's many configurations that are not needed for booting, yet they are 'default y'. Again, for historical reasons, yes, but do you see that happening for new code these days? And if it does get set to 'y' for new code, you have seen the yelling, right? Again, I'm totally confused as to _WHY_ this needs to be y. What is causing this oops without it? If an oops is happening, then shouldn't this be a strict dependancy? Why allow it to be disabled at all if it can break your box if you don't enable it? thanks, greg k-h -- 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] staging: tidspbridge: enable watchdog by default
On Thu, Feb 9, 2012 at 8:59 PM, Greg KH gre...@linuxfoundation.org wrote: On Thu, Feb 09, 2012 at 08:41:55PM +0200, Felipe Contreras wrote: On Thu, Feb 9, 2012 at 7:35 PM, Greg KH gre...@linuxfoundation.org wrote: On Wed, Feb 01, 2012 at 09:26:16AM +0200, Felipe Contreras wrote: On Wed, Feb 1, 2012 at 5:44 AM, Greg KH g...@kroah.com wrote: On Wed, Feb 01, 2012 at 03:37:55AM +0200, Felipe Contreras wrote: On Wed, Feb 1, 2012 at 3:12 AM, Greg KH gre...@suse.de wrote: On Wed, Feb 01, 2012 at 03:01:32AM +0200, Felipe Contreras wrote: From: Felipe Contreras felipe.contre...@nokia.com The public images have it enabled, it's safer, and we get rid of this warning: WARNING: at arch/arm/mach-omap2/omap_l3_smx.c:162 omap3_l3_app_irq+0x114/0x15c() In-band Error seen by IVA_SS at address 0 Modules linked in: bridgedriver(C+) mailbox_mach mailbox ipv6 g_ether [c0012848] (unwind_backtrace+0x0/0xec) from [c002e760] (warn_slowpath_common+0x4c/0x64) [c002e760] (warn_slowpath_common+0x4c/0x64) from [c002e7f8] (warn_slowpath_fmt+0x2c/0x3c) [c002e7f8] (warn_slowpath_fmt+0x2c/0x3c) from [c00217c8] (omap3_l3_app_irq+0x114/0x15c) [c00217c8] (omap3_l3_app_irq+0x114/0x15c) from [c0059844] (handle_irq_event_percpu+0x28/0x174) [c0059844] (handle_irq_event_percpu+0x28/0x174) from [c00599b8] (handle_irq_event+0x28/0x38) [c00599b8] (handle_irq_event+0x28/0x38) from [c005b8d0] (handle_level_irq+0xb8/0xe0) [c005b8d0] (handle_level_irq+0xb8/0xe0) from [c0059598] (generic_handle_irq+0x28/0x30) [c0059598] (generic_handle_irq+0x28/0x30) from [c000e62c] (handle_IRQ+0x60/0x84) [c000e62c] (handle_IRQ+0x60/0x84) from [c000d334] (__irq_svc+0x34/0x80) [c000d334] (__irq_svc+0x34/0x80) from [c0015804] (v7_dma_inv_range+0x34/0x48) [c0015804] (v7_dma_inv_range+0x34/0x48) from [c00129ec] (dma_cache_maint_page+0x2c/0x34) [c00129ec] (dma_cache_maint_page+0x2c/0x34) from [c0012a34] (___dma_page_dev_to_cpu+0x24/0x68) [c0012a34] (___dma_page_dev_to_cpu+0x24/0x68) from [c0012b50] (dma_unmap_sg+0x40/0x64) [c0012b50] (dma_unmap_sg+0x40/0x64) from [c019fba4] (omap_hsmmc_post_req+0x3c/0x48) [c019fba4] (omap_hsmmc_post_req+0x3c/0x48) from [c01932f4] (mmc_post_req+0x18/0x1c) [c01932f4] (mmc_post_req+0x18/0x1c) from [c019535c] (mmc_start_req+0xd4/0xec) [c019535c] (mmc_start_req+0xd4/0xec) from [c019d330] (mmc_blk_issue_rw_rq+0x64/0x244) [c019d330] (mmc_blk_issue_rw_rq+0x64/0x244) from [c019d7bc] (mmc_blk_issue_rq+0x2ac/0x2cc) [c019d7bc] (mmc_blk_issue_rq+0x2ac/0x2cc) from [c019e1c0] (mmc_queue_thread+0x8c/0xf8) [c019e1c0] (mmc_queue_thread+0x8c/0xf8) from [c0044c10] (kthread+0x84/0x8c) [c0044c10] (kthread+0x84/0x8c) from [c000e694] (kernel_thread_exit+0x0/0x8) ---[ end trace 5dec1c8d7857375d ]--- Signed-off-by: Felipe Contreras felipe.contre...@nokia.com --- drivers/staging/tidspbridge/Kconfig | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/staging/tidspbridge/Kconfig b/drivers/staging/tidspbridge/Kconfig index 21a559e..614e974 100644 --- a/drivers/staging/tidspbridge/Kconfig +++ b/drivers/staging/tidspbridge/Kconfig @@ -61,6 +61,7 @@ config TIDSPBRIDGE_CACHE_LINE_CHECK config TIDSPBRIDGE_WDT3 bool Enable watchdog timer depends on TIDSPBRIDGE + default y Nothing is ever default y, sorry, I can't accept this. % git grep 'default y' -- 'drivers/*/Kconfig' | wc -l 493 That's nice, but proves nothing, no new entries should be y. If it should be y, then it should just not be an option and always enabled, right? Why not do that? What if I compiled a DSP baseimage that doesn't have this watchdog? I don't know, you tell me. Then you want this option disabled. And using that logic, we should start removing all configurations in the kernel that have 'default y'. If your machine needs it to boot, then no, do not. There's many configurations that are not needed for booting, yet they are 'default y'. Again, for historical reasons, yes, but do you see that happening for new code these days? Yes, I already explained why, and I already pointed to a link where Linus says it's the right thing to do. Here it is again: http://article.gmane.org/gmane.linux.kernel/995419 It is needed to simplify defconfigs. And if it does get set to 'y' for new code, you have seen the yelling, right? I didn't parse that. Again, I'm totally confused as to _WHY_ this needs to be y. What is causing this oops without it? If an oops is happening, then shouldn't this be a strict dependancy? Why allow it to be disabled at all if it can break your box if you don't enable it? It's not an oops, it's a warning, and again, it depends on the firmware being used. We don't have
Re: [PATCH] staging: tidspbridge: enable watchdog by default
Hi, On Thu, Feb 9, 2012 at 3:30 PM, Felipe Contreras felipe.contre...@gmail.com wrote: Again, I'm totally confused as to _WHY_ this needs to be y. What is causing this oops without it? If an oops is happening, then shouldn't this be a strict dependancy? Why allow it to be disabled at all if it can break your box if you don't enable it? It's not an oops, it's a warning, and again, it depends on the firmware being used. We don't have control over that, and we have no way to detect if this feature is there. It's up to the user. I have been thinking more into it, how about looking for a WDT symbol inside the baseimage to decide whether to turn ON/OFF WDT3, this would mean that the code is always compiled in, but the decision to turn it on/off is made at runtime. Regards, Omar -- 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] staging: tidspbridge: enable watchdog by default
On Thu, Feb 09, 2012 at 04:45:00PM -0800, Ramirez Luna, Omar wrote: Hi, On Thu, Feb 9, 2012 at 3:30 PM, Felipe Contreras felipe.contre...@gmail.com wrote: Again, I'm totally confused as to _WHY_ this needs to be y. What is causing this oops without it? If an oops is happening, then shouldn't this be a strict dependancy? Why allow it to be disabled at all if it can break your box if you don't enable it? It's not an oops, it's a warning, and again, it depends on the firmware being used. We don't have control over that, and we have no way to detect if this feature is there. It's up to the user. I have been thinking more into it, how about looking for a WDT symbol inside the baseimage to decide whether to turn ON/OFF WDT3, this would mean that the code is always compiled in, but the decision to turn it on/off is made at runtime. I totally don't understand, why not just silence the warning properly then? I fail to understand why this warning happens, why it depends on the firmware, and why you can't detect it at runtime to not do it. And how it all ties into a kconfig option... confused, greg k-h -- 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] staging: tidspbridge: enable watchdog by default
On Wed, Feb 1, 2012 at 5:22 AM, Justin P. Mattock justinmatt...@gmail.com wrote: not sure if I see this warning or not on my machine, but is there a fix for the warning? I would rather see that, than hide it with setting: default y (if that's what its doing!) No, there's no fix. IIRC Omar said it would not be easy. -- Felipe Contreras -- 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] staging: tidspbridge: enable watchdog by default
On Wed, Feb 1, 2012 at 2:11 AM, Felipe Contreras felipe.contre...@gmail.com wrote: On Wed, Feb 1, 2012 at 5:22 AM, Justin P. Mattock justinmatt...@gmail.com wrote: not sure if I see this warning or not on my machine, but is there a fix for the warning? I would rather see that, than hide it with setting: default y (if that's what its doing!) No, there's no fix. IIRC Omar said it would not be easy. Indeed, the ideal would be the ability to turn off the config option but on the dsp side (when compiling the baseimage), however the release package only contains binaries given that the source is not open. I have been exploring introducing a flag in the shared memory area which would require a new baseimage release. Regards, Omar -- 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] staging: tidspbridge: enable watchdog by default
On Wed, Feb 01, 2012 at 03:01:32AM +0200, Felipe Contreras wrote: From: Felipe Contreras felipe.contre...@nokia.com The public images have it enabled, it's safer, and we get rid of this warning: WARNING: at arch/arm/mach-omap2/omap_l3_smx.c:162 omap3_l3_app_irq+0x114/0x15c() In-band Error seen by IVA_SS at address 0 Modules linked in: bridgedriver(C+) mailbox_mach mailbox ipv6 g_ether [c0012848] (unwind_backtrace+0x0/0xec) from [c002e760] (warn_slowpath_common+0x4c/0x64) [c002e760] (warn_slowpath_common+0x4c/0x64) from [c002e7f8] (warn_slowpath_fmt+0x2c/0x3c) [c002e7f8] (warn_slowpath_fmt+0x2c/0x3c) from [c00217c8] (omap3_l3_app_irq+0x114/0x15c) [c00217c8] (omap3_l3_app_irq+0x114/0x15c) from [c0059844] (handle_irq_event_percpu+0x28/0x174) [c0059844] (handle_irq_event_percpu+0x28/0x174) from [c00599b8] (handle_irq_event+0x28/0x38) [c00599b8] (handle_irq_event+0x28/0x38) from [c005b8d0] (handle_level_irq+0xb8/0xe0) [c005b8d0] (handle_level_irq+0xb8/0xe0) from [c0059598] (generic_handle_irq+0x28/0x30) [c0059598] (generic_handle_irq+0x28/0x30) from [c000e62c] (handle_IRQ+0x60/0x84) [c000e62c] (handle_IRQ+0x60/0x84) from [c000d334] (__irq_svc+0x34/0x80) [c000d334] (__irq_svc+0x34/0x80) from [c0015804] (v7_dma_inv_range+0x34/0x48) [c0015804] (v7_dma_inv_range+0x34/0x48) from [c00129ec] (dma_cache_maint_page+0x2c/0x34) [c00129ec] (dma_cache_maint_page+0x2c/0x34) from [c0012a34] (___dma_page_dev_to_cpu+0x24/0x68) [c0012a34] (___dma_page_dev_to_cpu+0x24/0x68) from [c0012b50] (dma_unmap_sg+0x40/0x64) [c0012b50] (dma_unmap_sg+0x40/0x64) from [c019fba4] (omap_hsmmc_post_req+0x3c/0x48) [c019fba4] (omap_hsmmc_post_req+0x3c/0x48) from [c01932f4] (mmc_post_req+0x18/0x1c) [c01932f4] (mmc_post_req+0x18/0x1c) from [c019535c] (mmc_start_req+0xd4/0xec) [c019535c] (mmc_start_req+0xd4/0xec) from [c019d330] (mmc_blk_issue_rw_rq+0x64/0x244) [c019d330] (mmc_blk_issue_rw_rq+0x64/0x244) from [c019d7bc] (mmc_blk_issue_rq+0x2ac/0x2cc) [c019d7bc] (mmc_blk_issue_rq+0x2ac/0x2cc) from [c019e1c0] (mmc_queue_thread+0x8c/0xf8) [c019e1c0] (mmc_queue_thread+0x8c/0xf8) from [c0044c10] (kthread+0x84/0x8c) [c0044c10] (kthread+0x84/0x8c) from [c000e694] (kernel_thread_exit+0x0/0x8) ---[ end trace 5dec1c8d7857375d ]--- Signed-off-by: Felipe Contreras felipe.contre...@nokia.com --- drivers/staging/tidspbridge/Kconfig |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/staging/tidspbridge/Kconfig b/drivers/staging/tidspbridge/Kconfig index 21a559e..614e974 100644 --- a/drivers/staging/tidspbridge/Kconfig +++ b/drivers/staging/tidspbridge/Kconfig @@ -61,6 +61,7 @@ config TIDSPBRIDGE_CACHE_LINE_CHECK config TIDSPBRIDGE_WDT3 bool Enable watchdog timer depends on TIDSPBRIDGE + default y Nothing is ever default y, sorry, I can't accept this. -- 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] staging: tidspbridge: enable watchdog by default
On Wed, Feb 1, 2012 at 3:12 AM, Greg KH gre...@suse.de wrote: On Wed, Feb 01, 2012 at 03:01:32AM +0200, Felipe Contreras wrote: From: Felipe Contreras felipe.contre...@nokia.com The public images have it enabled, it's safer, and we get rid of this warning: WARNING: at arch/arm/mach-omap2/omap_l3_smx.c:162 omap3_l3_app_irq+0x114/0x15c() In-band Error seen by IVA_SS at address 0 Modules linked in: bridgedriver(C+) mailbox_mach mailbox ipv6 g_ether [c0012848] (unwind_backtrace+0x0/0xec) from [c002e760] (warn_slowpath_common+0x4c/0x64) [c002e760] (warn_slowpath_common+0x4c/0x64) from [c002e7f8] (warn_slowpath_fmt+0x2c/0x3c) [c002e7f8] (warn_slowpath_fmt+0x2c/0x3c) from [c00217c8] (omap3_l3_app_irq+0x114/0x15c) [c00217c8] (omap3_l3_app_irq+0x114/0x15c) from [c0059844] (handle_irq_event_percpu+0x28/0x174) [c0059844] (handle_irq_event_percpu+0x28/0x174) from [c00599b8] (handle_irq_event+0x28/0x38) [c00599b8] (handle_irq_event+0x28/0x38) from [c005b8d0] (handle_level_irq+0xb8/0xe0) [c005b8d0] (handle_level_irq+0xb8/0xe0) from [c0059598] (generic_handle_irq+0x28/0x30) [c0059598] (generic_handle_irq+0x28/0x30) from [c000e62c] (handle_IRQ+0x60/0x84) [c000e62c] (handle_IRQ+0x60/0x84) from [c000d334] (__irq_svc+0x34/0x80) [c000d334] (__irq_svc+0x34/0x80) from [c0015804] (v7_dma_inv_range+0x34/0x48) [c0015804] (v7_dma_inv_range+0x34/0x48) from [c00129ec] (dma_cache_maint_page+0x2c/0x34) [c00129ec] (dma_cache_maint_page+0x2c/0x34) from [c0012a34] (___dma_page_dev_to_cpu+0x24/0x68) [c0012a34] (___dma_page_dev_to_cpu+0x24/0x68) from [c0012b50] (dma_unmap_sg+0x40/0x64) [c0012b50] (dma_unmap_sg+0x40/0x64) from [c019fba4] (omap_hsmmc_post_req+0x3c/0x48) [c019fba4] (omap_hsmmc_post_req+0x3c/0x48) from [c01932f4] (mmc_post_req+0x18/0x1c) [c01932f4] (mmc_post_req+0x18/0x1c) from [c019535c] (mmc_start_req+0xd4/0xec) [c019535c] (mmc_start_req+0xd4/0xec) from [c019d330] (mmc_blk_issue_rw_rq+0x64/0x244) [c019d330] (mmc_blk_issue_rw_rq+0x64/0x244) from [c019d7bc] (mmc_blk_issue_rq+0x2ac/0x2cc) [c019d7bc] (mmc_blk_issue_rq+0x2ac/0x2cc) from [c019e1c0] (mmc_queue_thread+0x8c/0xf8) [c019e1c0] (mmc_queue_thread+0x8c/0xf8) from [c0044c10] (kthread+0x84/0x8c) [c0044c10] (kthread+0x84/0x8c) from [c000e694] (kernel_thread_exit+0x0/0x8) ---[ end trace 5dec1c8d7857375d ]--- Signed-off-by: Felipe Contreras felipe.contre...@nokia.com --- drivers/staging/tidspbridge/Kconfig | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/staging/tidspbridge/Kconfig b/drivers/staging/tidspbridge/Kconfig index 21a559e..614e974 100644 --- a/drivers/staging/tidspbridge/Kconfig +++ b/drivers/staging/tidspbridge/Kconfig @@ -61,6 +61,7 @@ config TIDSPBRIDGE_CACHE_LINE_CHECK config TIDSPBRIDGE_WDT3 bool Enable watchdog timer depends on TIDSPBRIDGE + default y Nothing is ever default y, sorry, I can't accept this. % git grep 'default y' -- 'drivers/*/Kconfig' | wc -l 493 -- Felipe Contreras -- 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] staging: tidspbridge: enable watchdog by default
On Wed, Feb 01, 2012 at 03:37:55AM +0200, Felipe Contreras wrote: On Wed, Feb 1, 2012 at 3:12 AM, Greg KH gre...@suse.de wrote: On Wed, Feb 01, 2012 at 03:01:32AM +0200, Felipe Contreras wrote: From: Felipe Contreras felipe.contre...@nokia.com The public images have it enabled, it's safer, and we get rid of this warning: WARNING: at arch/arm/mach-omap2/omap_l3_smx.c:162 omap3_l3_app_irq+0x114/0x15c() In-band Error seen by IVA_SS at address 0 Modules linked in: bridgedriver(C+) mailbox_mach mailbox ipv6 g_ether [c0012848] (unwind_backtrace+0x0/0xec) from [c002e760] (warn_slowpath_common+0x4c/0x64) [c002e760] (warn_slowpath_common+0x4c/0x64) from [c002e7f8] (warn_slowpath_fmt+0x2c/0x3c) [c002e7f8] (warn_slowpath_fmt+0x2c/0x3c) from [c00217c8] (omap3_l3_app_irq+0x114/0x15c) [c00217c8] (omap3_l3_app_irq+0x114/0x15c) from [c0059844] (handle_irq_event_percpu+0x28/0x174) [c0059844] (handle_irq_event_percpu+0x28/0x174) from [c00599b8] (handle_irq_event+0x28/0x38) [c00599b8] (handle_irq_event+0x28/0x38) from [c005b8d0] (handle_level_irq+0xb8/0xe0) [c005b8d0] (handle_level_irq+0xb8/0xe0) from [c0059598] (generic_handle_irq+0x28/0x30) [c0059598] (generic_handle_irq+0x28/0x30) from [c000e62c] (handle_IRQ+0x60/0x84) [c000e62c] (handle_IRQ+0x60/0x84) from [c000d334] (__irq_svc+0x34/0x80) [c000d334] (__irq_svc+0x34/0x80) from [c0015804] (v7_dma_inv_range+0x34/0x48) [c0015804] (v7_dma_inv_range+0x34/0x48) from [c00129ec] (dma_cache_maint_page+0x2c/0x34) [c00129ec] (dma_cache_maint_page+0x2c/0x34) from [c0012a34] (___dma_page_dev_to_cpu+0x24/0x68) [c0012a34] (___dma_page_dev_to_cpu+0x24/0x68) from [c0012b50] (dma_unmap_sg+0x40/0x64) [c0012b50] (dma_unmap_sg+0x40/0x64) from [c019fba4] (omap_hsmmc_post_req+0x3c/0x48) [c019fba4] (omap_hsmmc_post_req+0x3c/0x48) from [c01932f4] (mmc_post_req+0x18/0x1c) [c01932f4] (mmc_post_req+0x18/0x1c) from [c019535c] (mmc_start_req+0xd4/0xec) [c019535c] (mmc_start_req+0xd4/0xec) from [c019d330] (mmc_blk_issue_rw_rq+0x64/0x244) [c019d330] (mmc_blk_issue_rw_rq+0x64/0x244) from [c019d7bc] (mmc_blk_issue_rq+0x2ac/0x2cc) [c019d7bc] (mmc_blk_issue_rq+0x2ac/0x2cc) from [c019e1c0] (mmc_queue_thread+0x8c/0xf8) [c019e1c0] (mmc_queue_thread+0x8c/0xf8) from [c0044c10] (kthread+0x84/0x8c) [c0044c10] (kthread+0x84/0x8c) from [c000e694] (kernel_thread_exit+0x0/0x8) ---[ end trace 5dec1c8d7857375d ]--- Signed-off-by: Felipe Contreras felipe.contre...@nokia.com --- drivers/staging/tidspbridge/Kconfig | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/staging/tidspbridge/Kconfig b/drivers/staging/tidspbridge/Kconfig index 21a559e..614e974 100644 --- a/drivers/staging/tidspbridge/Kconfig +++ b/drivers/staging/tidspbridge/Kconfig @@ -61,6 +61,7 @@ config TIDSPBRIDGE_CACHE_LINE_CHECK config TIDSPBRIDGE_WDT3 bool Enable watchdog timer depends on TIDSPBRIDGE + default y Nothing is ever default y, sorry, I can't accept this. % git grep 'default y' -- 'drivers/*/Kconfig' | wc -l 493 That's nice, but proves nothing, no new entries should be y. If it should be y, then it should just not be an option and always enabled, right? Why not do that? greg k-h -- 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] staging: tidspbridge: enable watchdog by default
On Wed, Feb 1, 2012 at 5:44 AM, Greg KH g...@kroah.com wrote: On Wed, Feb 01, 2012 at 03:37:55AM +0200, Felipe Contreras wrote: On Wed, Feb 1, 2012 at 3:12 AM, Greg KH gre...@suse.de wrote: On Wed, Feb 01, 2012 at 03:01:32AM +0200, Felipe Contreras wrote: From: Felipe Contreras felipe.contre...@nokia.com The public images have it enabled, it's safer, and we get rid of this warning: WARNING: at arch/arm/mach-omap2/omap_l3_smx.c:162 omap3_l3_app_irq+0x114/0x15c() In-band Error seen by IVA_SS at address 0 Modules linked in: bridgedriver(C+) mailbox_mach mailbox ipv6 g_ether [c0012848] (unwind_backtrace+0x0/0xec) from [c002e760] (warn_slowpath_common+0x4c/0x64) [c002e760] (warn_slowpath_common+0x4c/0x64) from [c002e7f8] (warn_slowpath_fmt+0x2c/0x3c) [c002e7f8] (warn_slowpath_fmt+0x2c/0x3c) from [c00217c8] (omap3_l3_app_irq+0x114/0x15c) [c00217c8] (omap3_l3_app_irq+0x114/0x15c) from [c0059844] (handle_irq_event_percpu+0x28/0x174) [c0059844] (handle_irq_event_percpu+0x28/0x174) from [c00599b8] (handle_irq_event+0x28/0x38) [c00599b8] (handle_irq_event+0x28/0x38) from [c005b8d0] (handle_level_irq+0xb8/0xe0) [c005b8d0] (handle_level_irq+0xb8/0xe0) from [c0059598] (generic_handle_irq+0x28/0x30) [c0059598] (generic_handle_irq+0x28/0x30) from [c000e62c] (handle_IRQ+0x60/0x84) [c000e62c] (handle_IRQ+0x60/0x84) from [c000d334] (__irq_svc+0x34/0x80) [c000d334] (__irq_svc+0x34/0x80) from [c0015804] (v7_dma_inv_range+0x34/0x48) [c0015804] (v7_dma_inv_range+0x34/0x48) from [c00129ec] (dma_cache_maint_page+0x2c/0x34) [c00129ec] (dma_cache_maint_page+0x2c/0x34) from [c0012a34] (___dma_page_dev_to_cpu+0x24/0x68) [c0012a34] (___dma_page_dev_to_cpu+0x24/0x68) from [c0012b50] (dma_unmap_sg+0x40/0x64) [c0012b50] (dma_unmap_sg+0x40/0x64) from [c019fba4] (omap_hsmmc_post_req+0x3c/0x48) [c019fba4] (omap_hsmmc_post_req+0x3c/0x48) from [c01932f4] (mmc_post_req+0x18/0x1c) [c01932f4] (mmc_post_req+0x18/0x1c) from [c019535c] (mmc_start_req+0xd4/0xec) [c019535c] (mmc_start_req+0xd4/0xec) from [c019d330] (mmc_blk_issue_rw_rq+0x64/0x244) [c019d330] (mmc_blk_issue_rw_rq+0x64/0x244) from [c019d7bc] (mmc_blk_issue_rq+0x2ac/0x2cc) [c019d7bc] (mmc_blk_issue_rq+0x2ac/0x2cc) from [c019e1c0] (mmc_queue_thread+0x8c/0xf8) [c019e1c0] (mmc_queue_thread+0x8c/0xf8) from [c0044c10] (kthread+0x84/0x8c) [c0044c10] (kthread+0x84/0x8c) from [c000e694] (kernel_thread_exit+0x0/0x8) ---[ end trace 5dec1c8d7857375d ]--- Signed-off-by: Felipe Contreras felipe.contre...@nokia.com --- drivers/staging/tidspbridge/Kconfig | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/staging/tidspbridge/Kconfig b/drivers/staging/tidspbridge/Kconfig index 21a559e..614e974 100644 --- a/drivers/staging/tidspbridge/Kconfig +++ b/drivers/staging/tidspbridge/Kconfig @@ -61,6 +61,7 @@ config TIDSPBRIDGE_CACHE_LINE_CHECK config TIDSPBRIDGE_WDT3 bool Enable watchdog timer depends on TIDSPBRIDGE + default y Nothing is ever default y, sorry, I can't accept this. % git grep 'default y' -- 'drivers/*/Kconfig' | wc -l 493 That's nice, but proves nothing, no new entries should be y. If it should be y, then it should just not be an option and always enabled, right? Why not do that? What if I compiled a DSP baseimage that doesn't have this watchdog? And using that logic, we should start removing all configurations in the kernel that have 'default y'. No. There's a reason they are *configurations*, and there's a reason they are 'default y'. You might have noticed Linus' rant about ARM defconfigs (LWN article[1]), and the solution to that was simplified configs (make savedefconfig), and the ideal would be to have no defconfigs at all. But in order to achieve that ideal goal (or at least move in that direction), the Kconfig files need to have good defaults--this was also agreed in the discussion. If you look at arch/arm/configs/, there are still quite a lot of configs there, and many not small at all. If Linus goes forward with his threat to remove ARM defconfigs, we would be in a bit of a situation, specially since there's still a lot of Kconfigs that have not been fixed. There's plenty of other reasons why 'default y' is good. This change is good, it helps everyone, and it doesn't hurt anyone. But if you like, I can put forward a case and take it with the penguin chief. [1] http://lwn.net/Articles/391372/ -- Felipe Contreras -- 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