RE: [PATCH] irqchip: gic: Don't complain in gic_get_cpumask() if UP system
Hi, On Sat, Jul 06, 2013 at 05:09:33, Stephen Boyd wrote: > In a uniprocessor implementation the interrupt processor targets > registers are read-as-zero/write-ignored (RAZ/WI). Unfortunately > gic_get_cpumask() will print a critical message saying > > GIC CPU mask not found - kernel will fail to boot. > > if these registers all read as zero, but there won't actually be > a problem on uniprocessor systems and the kernel will boot just > fine. Skip this check if we're running a UP kernel or if we > detect that the hardware only supports a single processor. > > Cc: Nicolas Pitre > Cc: Russell King > Signed-off-by: Stephen Boyd > --- > > Maybe we should just drop the check entirely? It looks like it may > just be debug code that won't ever trigger in practice, even on the > 11MPCore that caused this code to be introduced. > Apologies if I missed v2 of this patch if there was one. I could not locate it in my inbox or the list archives. We have a yet to be released single-core A9 MPCore system (AM437x) which needs to co-exist in omap2plus_defconfig wherein NR_CPUS is set to 2. This currently leads to the "GIC CPU mask not found..." being printed twice during bootup. If the check is really some debug code, can it be completely dropped? Regards, Vaibhav B. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] irqchip: gic: Don't complain in gic_get_cpumask() if UP system
Hi, On Sat, Jul 06, 2013 at 05:09:33, Stephen Boyd wrote: In a uniprocessor implementation the interrupt processor targets registers are read-as-zero/write-ignored (RAZ/WI). Unfortunately gic_get_cpumask() will print a critical message saying GIC CPU mask not found - kernel will fail to boot. if these registers all read as zero, but there won't actually be a problem on uniprocessor systems and the kernel will boot just fine. Skip this check if we're running a UP kernel or if we detect that the hardware only supports a single processor. Cc: Nicolas Pitre n...@linaro.org Cc: Russell King rmk+ker...@arm.linux.org.uk Signed-off-by: Stephen Boyd sb...@codeaurora.org --- Maybe we should just drop the check entirely? It looks like it may just be debug code that won't ever trigger in practice, even on the 11MPCore that caused this code to be introduced. Apologies if I missed v2 of this patch if there was one. I could not locate it in my inbox or the list archives. We have a yet to be released single-core A9 MPCore system (AM437x) which needs to co-exist in omap2plus_defconfig wherein NR_CPUS is set to 2. This currently leads to the GIC CPU mask not found... being printed twice during bootup. If the check is really some debug code, can it be completely dropped? Regards, Vaibhav B. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [RFC PATCH 0/2] cpufreq/regulator: Limit minimum voltage only
(removing Anil's email-id since it's no longer valid) On Sat, Apr 20, 2013 at 05:54:10, Kondratiuk, Taras wrote: > On 04/19/2013 07:21 PM, Nishanth Menon wrote: > > On 14:55-20130419, Taras Kondratiuk wrote: > >> Using a "voltage tolerance" for doing DVFS is not a proper way. > >> It leads to a few issues: > >> - voltage is limited to a narrow range near OPP voltage, > >> so other consumers of the same regulator can't set their own constraints > >> if they don't overlap with this narrow range. No ganged rails :( > >> - usually OPP voltage is an absolute minimum voltage > >> necessary for correct work (not taking into account AVS). The absolute minimum voltage part is not applicable for all SoCs. In case of AM335x there a nominal voltage that is specified in the OPP table and there's a plus/minus tolerance at the IO level within which things are guaranteed to work. To lower the power consumption we would want to go as low as possible without violating the requirement at the SoC boundary level. I don't know how the OPP voltages are speced for non TI devices but if there's a permissible range I guess everyone would like to set the voltage near the lower end of the spectrum. > >> Applying plus/minus tolerance can lead to an unstable device. > >> For example omap-cpufreq has 4% tolerance configured, > >> so for OMAP4430 MPU OPP50 we get 0.984V instead of 1.025V. So if you set the tolerance to 0% instead of removing it completely won't the problem go away? > > there is a reason for this - board level IRDROP and the real PMIC > > accuracy compared to the SoC assumption about the PMIC accuracy. > > I don't see how current implementation of voltage-tolerance can help with > this. > It does help (more below). > > > > That said, I had been always been a little confused with the usage > > in AM335x. For that matter, I dont even think this is constrainted to > > TI SoC usage, other SoCs also probably have the same pain. > > > > How does it actually work? > > I think this is a question to Afzal as he is an original author > of commit 42daffd2d6c665716d442d518022ecaad17ddf64 which later > migrated to cpufreq-cpu0 driver. > > I can only guess... > Without tolerance cpufreq requests the same value for min_uV and max_uV. > So regulator have to set an exact voltage value on the rail, > which is not always possible if different PMICs can be used for the SoC. > For example in v3.9-rc7 voltage-tolerance is used *only* > in AM33xx which can use two PMICs: TPS65217 and TPS65910. > These PMICs have different voltage steps so they can't set the same voltage. > I think this was the reason for adding voltage-tolerance. > The PMICs have the same step size of 12.5mV but unfortunately they don't have a register setting to meet the nominal voltage requirement for all the OPPs. As per the SoC datasheet, for OPP120 the nominal voltage is 1.26V but the closest that the PMIC outputs can come is either 1.25V or 1.275V. Now i think there's been some confusion in the implementation phase due to things like board level IR drops and variations mentioned in the PMIC datasheets which has resulted in the tolerance being used both in am33xx.dtsi and then again in the cpufreq driver. Ignoring the PMIC variations and board level IR drops for moment, the way I think it should have been done is - OPP entries have the nominal voltages and we specify the tolerance either in % in the DT file. The cpufreq driver looks up the nominal voltage from the OPP table and then requests the regulator framework to factor in the tolerance and then select the lowest permissible voltage. > *But* there is a trick. > If you compare MPU OPP voltages in AM335x datasheet and am33xx.dtsi > you will see that am33xx.dtsi has already applied tolerance (2%) on top > of nominal voltages. > So final range passed to regulator is [Vnom; Vnom+2*tol] > instead of [Vnom-tol; Vnom+tol]. > That's why it works for AM335x, but IMHO it is a hack. I agree. It sort of ended up a hack that needs to be fixed by removing the additional tolerance in am33xx.dtsi. > That patch broke omap-cpufreq functionality, since mach-omap2/opp_data.c > files doesn't have tolerance added on top of nominal voltages. > Again, if you specify 0% as the tolerance this would be fine, no? > regulator_set_voltage_min() should solve AM335x issue > without introducing voltage-tolerance hack. No, we want to pass on all the factors to be considered to the appropriate framework which in this case is the regulator framework and let it decide the min voltage. If you consider multiple users like ABB and AVS, I think it makes much more sense to have the requirements from the different users being passed on to the regulator framework and letting it decide what works for all of them. > Probably I need to add one more patch to the series which will remove > voltage-tolerance from am335x.dtsi and set CPU voltages back to nominal. > > > > > Lets say ntarget/Vnom has PMIC tolerance of 5%
RE: [RFC PATCH 0/2] cpufreq/regulator: Limit minimum voltage only
(removing Anil's email-id since it's no longer valid) On Sat, Apr 20, 2013 at 05:54:10, Kondratiuk, Taras wrote: On 04/19/2013 07:21 PM, Nishanth Menon wrote: On 14:55-20130419, Taras Kondratiuk wrote: Using a voltage tolerance for doing DVFS is not a proper way. It leads to a few issues: - voltage is limited to a narrow range near OPP voltage, so other consumers of the same regulator can't set their own constraints if they don't overlap with this narrow range. No ganged rails :( - usually OPP voltage is an absolute minimum voltage necessary for correct work (not taking into account AVS). The absolute minimum voltage part is not applicable for all SoCs. In case of AM335x there a nominal voltage that is specified in the OPP table and there's a plus/minus tolerance at the IO level within which things are guaranteed to work. To lower the power consumption we would want to go as low as possible without violating the requirement at the SoC boundary level. I don't know how the OPP voltages are speced for non TI devices but if there's a permissible range I guess everyone would like to set the voltage near the lower end of the spectrum. Applying plus/minus tolerance can lead to an unstable device. For example omap-cpufreq has 4% tolerance configured, so for OMAP4430 MPU OPP50 we get 0.984V instead of 1.025V. So if you set the tolerance to 0% instead of removing it completely won't the problem go away? there is a reason for this - board level IRDROP and the real PMIC accuracy compared to the SoC assumption about the PMIC accuracy. I don't see how current implementation of voltage-tolerance can help with this. It does help (more below). That said, I had been always been a little confused with the usage in AM335x. For that matter, I dont even think this is constrainted to TI SoC usage, other SoCs also probably have the same pain. How does it actually work? I think this is a question to Afzal as he is an original author of commit 42daffd2d6c665716d442d518022ecaad17ddf64 which later migrated to cpufreq-cpu0 driver. I can only guess... Without tolerance cpufreq requests the same value for min_uV and max_uV. So regulator have to set an exact voltage value on the rail, which is not always possible if different PMICs can be used for the SoC. For example in v3.9-rc7 voltage-tolerance is used *only* in AM33xx which can use two PMICs: TPS65217 and TPS65910. These PMICs have different voltage steps so they can't set the same voltage. I think this was the reason for adding voltage-tolerance. The PMICs have the same step size of 12.5mV but unfortunately they don't have a register setting to meet the nominal voltage requirement for all the OPPs. As per the SoC datasheet, for OPP120 the nominal voltage is 1.26V but the closest that the PMIC outputs can come is either 1.25V or 1.275V. Now i think there's been some confusion in the implementation phase due to things like board level IR drops and variations mentioned in the PMIC datasheets which has resulted in the tolerance being used both in am33xx.dtsi and then again in the cpufreq driver. Ignoring the PMIC variations and board level IR drops for moment, the way I think it should have been done is - OPP entries have the nominal voltages and we specify the tolerance either in % in the DT file. The cpufreq driver looks up the nominal voltage from the OPP table and then requests the regulator framework to factor in the tolerance and then select the lowest permissible voltage. *But* there is a trick. If you compare MPU OPP voltages in AM335x datasheet and am33xx.dtsi you will see that am33xx.dtsi has already applied tolerance (2%) on top of nominal voltages. So final range passed to regulator is [Vnom; Vnom+2*tol] instead of [Vnom-tol; Vnom+tol]. That's why it works for AM335x, but IMHO it is a hack. I agree. It sort of ended up a hack that needs to be fixed by removing the additional tolerance in am33xx.dtsi. That patch broke omap-cpufreq functionality, since mach-omap2/opp_data.c files doesn't have tolerance added on top of nominal voltages. Again, if you specify 0% as the tolerance this would be fine, no? regulator_set_voltage_min() should solve AM335x issue without introducing voltage-tolerance hack. No, we want to pass on all the factors to be considered to the appropriate framework which in this case is the regulator framework and let it decide the min voltage. If you consider multiple users like ABB and AVS, I think it makes much more sense to have the requirements from the different users being passed on to the regulator framework and letting it decide what works for all of them. Probably I need to add one more patch to the series which will remove voltage-tolerance from am335x.dtsi and set CPU voltages back to nominal. Lets say ntarget/Vnom has PMIC tolerance of 5% (SoC tolerance assumed for OPP voltage documented in data manual for the SoC), say the
RE: [PATCHv3] driver: serial: prevent UART console idle on suspend while using "no_console_suspend"
Hi Kevin, On Thu, Apr 11, 2013 at 19:45:33, Kevin Hilman wrote: > Kevin Hilman writes: > > > "Bedia, Vaibhav" writes: > > > >> Hi Sourav, > >> > >> On Wed, Apr 10, 2013 at 15:13:44, Poddar, Sourav wrote: > >> [...] > >>> Yes, had a look at that and found your situation similar to UART. > >>> > >>> But how exactly this gets used, I mean I don't see any drivers/ in > >>> mainline > >>> making use of this compatible string "ti,am3352-ocmcram". ? > >> > >> OCMC clock is enabled during bootup (not sure whether that's the h/w > >> default or ROM does it) since the initial bootloader runs from there. > >> By marking the corresponding hwmod with HWMOD_INIT_NO_IDLE we leave the > >> clock running. Right now the sram code under arch/arm/plat-omap/ is what > >> manages the OCMC. I guess this needs to move somewhere under drivers/ > >> and start managing the clocks. Even then we'll need a mechanism > >> to leave the clocks running as part of the kernel suspend process > >> since the assembly code which runs at the fag end of the suspend > >> process runs out of OCMC and hence we can't cut its clock. > >> > >> On AM335x, the OCMC clock is cut to have PER power domain transition > >> but that's done in the WKUP-M3 firmware when going down. During the > >> wakeup sequence, WKUP-M3 re-enables the OCMC clock so that when the > >> kernel resumes the h/w state is same. > > > > OK, but *today*, in *mainline*, where in the linux kernel (not the M3 > > firmware) is the OCMRAM clock cut during suspend? > > > > From what I can see, there is no driver for this device, so there are no > > system PM calls being done for that device, and thus no omap_device > > calls being done for that device, so the no_idle_on_suspend has no > > effect. > > OK, I think I confused things here, sorry. I was thinking runtime PM > here, but wrote system PM. The no_idle_on_suspend feature only affects > system PM, and the omap_device calls will still be called during system > PM, even without a driver. > > That being said, the commit below[1], added in v3.6 should prevent the > any automaic clock gating for devices without drivers bound. Since > there is no driver for the OCM RAM block, you shouldn't be affected by > the automatic idle on suspend anyways. > > So, my proposal is that Sourav remove that flag from the AM33xx hwmod > when he removes this feature. > Apologies for the delayed response. I was out of office for a couple of days. I don't recall the exact kernel version in which I ended up adding this flag to keep the clock running but yes after the change mentioned below this flag is not required. I just did a sanity check by removing this flag on v3.8 kernel and things work fine across suspend. Regards, Vaibhav -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCHv3] driver: serial: prevent UART console idle on suspend while using no_console_suspend
Hi Kevin, On Thu, Apr 11, 2013 at 19:45:33, Kevin Hilman wrote: Kevin Hilman khil...@linaro.org writes: Bedia, Vaibhav vaibhav.be...@ti.com writes: Hi Sourav, On Wed, Apr 10, 2013 at 15:13:44, Poddar, Sourav wrote: [...] Yes, had a look at that and found your situation similar to UART. But how exactly this gets used, I mean I don't see any drivers/ in mainline making use of this compatible string ti,am3352-ocmcram. ? OCMC clock is enabled during bootup (not sure whether that's the h/w default or ROM does it) since the initial bootloader runs from there. By marking the corresponding hwmod with HWMOD_INIT_NO_IDLE we leave the clock running. Right now the sram code under arch/arm/plat-omap/ is what manages the OCMC. I guess this needs to move somewhere under drivers/ and start managing the clocks. Even then we'll need a mechanism to leave the clocks running as part of the kernel suspend process since the assembly code which runs at the fag end of the suspend process runs out of OCMC and hence we can't cut its clock. On AM335x, the OCMC clock is cut to have PER power domain transition but that's done in the WKUP-M3 firmware when going down. During the wakeup sequence, WKUP-M3 re-enables the OCMC clock so that when the kernel resumes the h/w state is same. OK, but *today*, in *mainline*, where in the linux kernel (not the M3 firmware) is the OCMRAM clock cut during suspend? From what I can see, there is no driver for this device, so there are no system PM calls being done for that device, and thus no omap_device calls being done for that device, so the no_idle_on_suspend has no effect. OK, I think I confused things here, sorry. I was thinking runtime PM here, but wrote system PM. The no_idle_on_suspend feature only affects system PM, and the omap_device calls will still be called during system PM, even without a driver. That being said, the commit below[1], added in v3.6 should prevent the any automaic clock gating for devices without drivers bound. Since there is no driver for the OCM RAM block, you shouldn't be affected by the automatic idle on suspend anyways. So, my proposal is that Sourav remove that flag from the AM33xx hwmod when he removes this feature. Apologies for the delayed response. I was out of office for a couple of days. I don't recall the exact kernel version in which I ended up adding this flag to keep the clock running but yes after the change mentioned below this flag is not required. I just did a sanity check by removing this flag on v3.8 kernel and things work fine across suspend. Regards, Vaibhav -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCHv3] driver: serial: prevent UART console idle on suspend while using "no_console_suspend"
Hi Sourav, On Wed, Apr 10, 2013 at 15:13:44, Poddar, Sourav wrote: [...] > Yes, had a look at that and found your situation similar to UART. > > But how exactly this gets used, I mean I don't see any drivers/ in mainline > making use of this compatible string "ti,am3352-ocmcram". ? OCMC clock is enabled during bootup (not sure whether that's the h/w default or ROM does it) since the initial bootloader runs from there. By marking the corresponding hwmod with HWMOD_INIT_NO_IDLE we leave the clock running. Right now the sram code under arch/arm/plat-omap/ is what manages the OCMC. I guess this needs to move somewhere under drivers/ and start managing the clocks. Even then we'll need a mechanism to leave the clocks running as part of the kernel suspend process since the assembly code which runs at the fag end of the suspend process runs out of OCMC and hence we can't cut its clock. On AM335x, the OCMC clock is cut to have PER power domain transition but that's done in the WKUP-M3 firmware when going down. During the wakeup sequence, WKUP-M3 re-enables the OCMC clock so that when the kernel resumes the h/w state is same. Regards, Vaibhav -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCHv3] driver: serial: prevent UART console idle on suspend while using "no_console_suspend"
Hi Sourav, Kevin, On Wed, Apr 10, 2013 at 11:37:28, Poddar, Sourav wrote: > Hi, > On Wednesday 10 April 2013 12:37 AM, Kevin Hilman wrote: > > Sourav Poddar writes: > > > >> Hi Kevin, > >> On Friday 05 April 2013 11:10 PM, Kevin Hilman wrote: > >>> Sourav Poddar writes: > >>> > With dt boot, uart wakeup after suspend is non functional while using > "no_console_suspend" in the bootargs. With "no_console_suspend" used, we > should prevent the runtime suspend of the uart port which is getting used > as an console. > > Cc: Santosh Shilimkar > Cc: Felipe Balbi > Cc: Rajendra nayak > Tested on omap5430evm, omap4430sdp. > > Signed-off-by: Sourav Poddar > >>> Rather than make these special checks inside the driver's runtime PM > >>> callbacks, you should just disable runtime PM (pm_runtime_disable()) > >>> > >>> Then, this should be broken into 2 patches. > >>> > >>> 1) serial core: add the '->is_console' flag. (nit on naming: don't call > >>> it port_is_console, since the struct is already a uart_port) > >>> > >>> 2) In the OMAP UART driver's ->prepare callback, check the is_console flag > >>> and pm_runtime_disable() accordingly (then pm_runtime_enable() in > >>> the drivers's ->complete callback. > >>> > >>> Kevin > >> I was working on your above suggestions, but realised there is not > >> only console > >> uart which has the requirement of keeping the clocks enabled while going on > >> suspend. > >> > >> If you see arch/arm/boot/dts/am33xx.dtsi, there is a ocmcram which has > >> "no_idle_on_suspend" property used. > > Can you please ask the AM33xx folks how (and why) this is being used? > > > > I don't see/find a driver for this device in mainline, so without a > > driver this flag will not be used. > > > Looping in Vaibhav Bedia for ocmcram.. > > [Vaibhav]: >There is a discussion going on about a cleaner way of handling >ti, no_idle_on_suspend" part (as this is a sort of hack). We got a way >around for UART ($subject) by making serial core/driver handle this > for us. >But with this, we will delete codes around "no_idle_on_suspend" flag in >omap_device file. > >But, we realised that its not only UART which requires the clocks to > be active >whie going for suspend. There is a dts entry for ocmcram also. > >As Kevin also pointed out, we don't see a driver for this device in > mainline, It would be >great if you can explain how its getting used? > >You can find the complete discussion on v3 here: > https://lkml.org/lkml/2013/4/5/239 > The flag in question is used to ensure that the clock to OCMC RAM is not disabled by the PM code. >From the changelog which added this flag: "Note: OCMC RAM is part of the PER power domain and supports retention. The assembly code for low power entry/exit will run from OCMC RAM. To ensure that the OMAP PM code does not attempt to disable the clock to OCMC RAM as part of the suspend process add the no_idle_on_suspend flag." We had discussed about the usage of this flag in the RFC version of the patch [1]. Regards, Vaibhav [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-November/129510.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCHv3] driver: serial: prevent UART console idle on suspend while using no_console_suspend
Hi Sourav, Kevin, On Wed, Apr 10, 2013 at 11:37:28, Poddar, Sourav wrote: Hi, On Wednesday 10 April 2013 12:37 AM, Kevin Hilman wrote: Sourav Poddarsourav.pod...@ti.com writes: Hi Kevin, On Friday 05 April 2013 11:10 PM, Kevin Hilman wrote: Sourav Poddarsourav.pod...@ti.com writes: With dt boot, uart wakeup after suspend is non functional while using no_console_suspend in the bootargs. With no_console_suspend used, we should prevent the runtime suspend of the uart port which is getting used as an console. Cc: Santosh Shilimkarsantosh.shilim...@ti.com Cc: Felipe Balbiba...@ti.com Cc: Rajendra nayakrna...@ti.com Tested on omap5430evm, omap4430sdp. Signed-off-by: Sourav Poddarsourav.pod...@ti.com Rather than make these special checks inside the driver's runtime PM callbacks, you should just disable runtime PM (pm_runtime_disable()) Then, this should be broken into 2 patches. 1) serial core: add the '-is_console' flag. (nit on naming: don't call it port_is_console, since the struct is already a uart_port) 2) In the OMAP UART driver's -prepare callback, check the is_console flag and pm_runtime_disable() accordingly (then pm_runtime_enable() in the drivers's -complete callback. Kevin I was working on your above suggestions, but realised there is not only console uart which has the requirement of keeping the clocks enabled while going on suspend. If you see arch/arm/boot/dts/am33xx.dtsi, there is a ocmcram which has no_idle_on_suspend property used. Can you please ask the AM33xx folks how (and why) this is being used? I don't see/find a driver for this device in mainline, so without a driver this flag will not be used. Looping in Vaibhav Bedia for ocmcram.. [Vaibhav]: There is a discussion going on about a cleaner way of handling ti, no_idle_on_suspend part (as this is a sort of hack). We got a way around for UART ($subject) by making serial core/driver handle this for us. But with this, we will delete codes around no_idle_on_suspend flag in omap_device file. But, we realised that its not only UART which requires the clocks to be active whie going for suspend. There is a dts entry for ocmcram also. As Kevin also pointed out, we don't see a driver for this device in mainline, It would be great if you can explain how its getting used? You can find the complete discussion on v3 here: https://lkml.org/lkml/2013/4/5/239 The flag in question is used to ensure that the clock to OCMC RAM is not disabled by the PM code. From the changelog which added this flag: Note: OCMC RAM is part of the PER power domain and supports retention. The assembly code for low power entry/exit will run from OCMC RAM. To ensure that the OMAP PM code does not attempt to disable the clock to OCMC RAM as part of the suspend process add the no_idle_on_suspend flag. We had discussed about the usage of this flag in the RFC version of the patch [1]. Regards, Vaibhav [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-November/129510.html -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCHv3] driver: serial: prevent UART console idle on suspend while using no_console_suspend
Hi Sourav, On Wed, Apr 10, 2013 at 15:13:44, Poddar, Sourav wrote: [...] Yes, had a look at that and found your situation similar to UART. But how exactly this gets used, I mean I don't see any drivers/ in mainline making use of this compatible string ti,am3352-ocmcram. ? OCMC clock is enabled during bootup (not sure whether that's the h/w default or ROM does it) since the initial bootloader runs from there. By marking the corresponding hwmod with HWMOD_INIT_NO_IDLE we leave the clock running. Right now the sram code under arch/arm/plat-omap/ is what manages the OCMC. I guess this needs to move somewhere under drivers/ and start managing the clocks. Even then we'll need a mechanism to leave the clocks running as part of the kernel suspend process since the assembly code which runs at the fag end of the suspend process runs out of OCMC and hence we can't cut its clock. On AM335x, the OCMC clock is cut to have PER power domain transition but that's done in the WKUP-M3 firmware when going down. During the wakeup sequence, WKUP-M3 re-enables the OCMC clock so that when the kernel resumes the h/w state is same. Regards, Vaibhav -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] ARM: OMAP2+: am33xx: preserve JTAG clock aka debugss_ick.
On Thu, Mar 07, 2013 at 18:43:27, Andreas Fenkart wrote: > This fixes JTAG support on am33xx. > Please refer to http://www.spinics.net/lists/linux-omap/msg87476.html Regards, Vaibhav -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] ARM: OMAP2+: am33xx: preserve JTAG clock aka debugss_ick.
On Thu, Mar 07, 2013 at 18:43:27, Andreas Fenkart wrote: This fixes JTAG support on am33xx. Please refer to http://www.spinics.net/lists/linux-omap/msg87476.html Regards, Vaibhav -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] cpsw: Fix interrupt storm among other things
Hi Antoniou, On Wed, Jan 30, 2013 at 19:07:19, Pantelis Antoniou wrote: > Hi Vaibhav, > > On Jan 30, 2013, at 3:29 PM, Bedia, Vaibhav wrote: > > > On Wed, Jan 30, 2013 at 18:14:30, Pantelis Antoniou wrote: > >> Hi Vaibhav, > >> > >> On Jan 30, 2013, at 2:38 PM, Bedia, Vaibhav wrote: > >> > >>> On Wed, Jan 30, 2013 at 16:38:50, Pantelis Antoniou wrote: > >>> [...] > >>>> > >>>> TBH I haven't found a simple way to print out the silicon revision > >>>> number. > >>>> Anyone on the list know a quick and dirty method? > >>>> > >>> > >>> You can dump the DEVICE_ID register @ 0x44e10600. > >>> Bits 31:28 should be 0 for PG1.0 and 1 for PG2.0. > >>> > >> > >> Thanks this works perfectly: > >> > >> original-bone: > >> root@beaglebone:~# devmem2 0x44e10600 w > >> Read at address 0x44E10600 (0xb6ff4600): 0x0B94402E > >> > >> bone-black: > >> root@beaglebone:~# devmem2 0x44e10600 w > >> Read at address 0x44E10600 (0xb6fcc600): 0x1B94402E > >> > > > > I just re-read the mail-chain and I am confused here. > > So the patch in question is meant for Bone-A4 which has > > PG1.0? > > > > It is a general bug fix. The problem was discovered only on > the bone black which has PG2.0 silicon. The driver has been > tested and it works on the original bone with PG1.0 as well. > But Mugunthan mentioned that he doesn't see this on an EVM with PG2.0 silicon... is there any board dependency here? Regards, Vaibhav -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] cpsw: Fix interrupt storm among other things
On Wed, Jan 30, 2013 at 18:14:30, Pantelis Antoniou wrote: > Hi Vaibhav, > > On Jan 30, 2013, at 2:38 PM, Bedia, Vaibhav wrote: > > > On Wed, Jan 30, 2013 at 16:38:50, Pantelis Antoniou wrote: > > [...] > >> > >> TBH I haven't found a simple way to print out the silicon revision number. > >> Anyone on the list know a quick and dirty method? > >> > > > > You can dump the DEVICE_ID register @ 0x44e10600. > > Bits 31:28 should be 0 for PG1.0 and 1 for PG2.0. > > > > Thanks this works perfectly: > > original-bone: > root@beaglebone:~# devmem2 0x44e10600 w > Read at address 0x44E10600 (0xb6ff4600): 0x0B94402E > > bone-black: > root@beaglebone:~# devmem2 0x44e10600 w > Read at address 0x44E10600 (0xb6fcc600): 0x1B94402E > I just re-read the mail-chain and I am confused here. So the patch in question is meant for Bone-A4 which has PG1.0? Regards, Vaibhav -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] cpsw: Fix interrupt storm among other things
On Wed, Jan 30, 2013 at 16:38:50, Pantelis Antoniou wrote: [...] > > TBH I haven't found a simple way to print out the silicon revision number. > Anyone on the list know a quick and dirty method? > You can dump the DEVICE_ID register @ 0x44e10600. Bits 31:28 should be 0 for PG1.0 and 1 for PG2.0. Regards, Vaibhav -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] cpsw: Fix interrupt storm among other things
On Wed, Jan 30, 2013 at 16:38:50, Pantelis Antoniou wrote: [...] TBH I haven't found a simple way to print out the silicon revision number. Anyone on the list know a quick and dirty method? You can dump the DEVICE_ID register @ 0x44e10600. Bits 31:28 should be 0 for PG1.0 and 1 for PG2.0. Regards, Vaibhav -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] cpsw: Fix interrupt storm among other things
On Wed, Jan 30, 2013 at 18:14:30, Pantelis Antoniou wrote: Hi Vaibhav, On Jan 30, 2013, at 2:38 PM, Bedia, Vaibhav wrote: On Wed, Jan 30, 2013 at 16:38:50, Pantelis Antoniou wrote: [...] TBH I haven't found a simple way to print out the silicon revision number. Anyone on the list know a quick and dirty method? You can dump the DEVICE_ID register @ 0x44e10600. Bits 31:28 should be 0 for PG1.0 and 1 for PG2.0. Thanks this works perfectly: original-bone: root@beaglebone:~# devmem2 0x44e10600 w Read at address 0x44E10600 (0xb6ff4600): 0x0B94402E bone-black: root@beaglebone:~# devmem2 0x44e10600 w Read at address 0x44E10600 (0xb6fcc600): 0x1B94402E I just re-read the mail-chain and I am confused here. So the patch in question is meant for Bone-A4 which has PG1.0? Regards, Vaibhav -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH] cpsw: Fix interrupt storm among other things
Hi Antoniou, On Wed, Jan 30, 2013 at 19:07:19, Pantelis Antoniou wrote: Hi Vaibhav, On Jan 30, 2013, at 3:29 PM, Bedia, Vaibhav wrote: On Wed, Jan 30, 2013 at 18:14:30, Pantelis Antoniou wrote: Hi Vaibhav, On Jan 30, 2013, at 2:38 PM, Bedia, Vaibhav wrote: On Wed, Jan 30, 2013 at 16:38:50, Pantelis Antoniou wrote: [...] TBH I haven't found a simple way to print out the silicon revision number. Anyone on the list know a quick and dirty method? You can dump the DEVICE_ID register @ 0x44e10600. Bits 31:28 should be 0 for PG1.0 and 1 for PG2.0. Thanks this works perfectly: original-bone: root@beaglebone:~# devmem2 0x44e10600 w Read at address 0x44E10600 (0xb6ff4600): 0x0B94402E bone-black: root@beaglebone:~# devmem2 0x44e10600 w Read at address 0x44E10600 (0xb6fcc600): 0x1B94402E I just re-read the mail-chain and I am confused here. So the patch in question is meant for Bone-A4 which has PG1.0? It is a general bug fix. The problem was discovered only on the bone black which has PG2.0 silicon. The driver has been tested and it works on the original bone with PG1.0 as well. But Mugunthan mentioned that he doesn't see this on an EVM with PG2.0 silicon... is there any board dependency here? Regards, Vaibhav -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/9] mailbox: OMAP: introduce mailbox framework
On Wed, Jan 09, 2013 at 17:59:39, Loic PALLARDY wrote: > Hi Vaibhav, > > On 01/09/2013 01:11 PM, Bedia, Vaibhav wrote: > > Hi Loic, > > > > On Fri, Dec 21, 2012 at 16:23:24, Loic PALLARDY wrote: > >> > >> > >> On 12/21/2012 11:49 AM, Bedia, Vaibhav wrote: > >>> On Fri, Dec 21, 2012 at 14:24:26, Loic PALLARDY wrote: > >>>> > >>> I have a few patches which are dependent on this patch series. > >>> Could you please keep me in cc for the future versions. > >>> > >> Sure, I'll. > >> > > > > When do you plan to post an updated version of these patches? > I'm synchronizing with Omar to include TI RPMsg and tidspbridge patches > in next update. > So I plan update for end of the week, beginning of next week. > Ok. Thanks. Regards, Vaibhav -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/9] mailbox: OMAP: introduce mailbox framework
Hi Loic, On Fri, Dec 21, 2012 at 16:23:24, Loic PALLARDY wrote: > > > On 12/21/2012 11:49 AM, Bedia, Vaibhav wrote: > > On Fri, Dec 21, 2012 at 14:24:26, Loic PALLARDY wrote: > >> > > I have a few patches which are dependent on this patch series. > > Could you please keep me in cc for the future versions. > > > Sure, I'll. > When do you plan to post an updated version of these patches? Regards, Vaibhav -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/9] mailbox: OMAP: introduce mailbox framework
Hi Loic, On Fri, Dec 21, 2012 at 16:23:24, Loic PALLARDY wrote: On 12/21/2012 11:49 AM, Bedia, Vaibhav wrote: On Fri, Dec 21, 2012 at 14:24:26, Loic PALLARDY wrote: I have a few patches which are dependent on this patch series. Could you please keep me in cc for the future versions. Sure, I'll. When do you plan to post an updated version of these patches? Regards, Vaibhav -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/9] mailbox: OMAP: introduce mailbox framework
On Wed, Jan 09, 2013 at 17:59:39, Loic PALLARDY wrote: Hi Vaibhav, On 01/09/2013 01:11 PM, Bedia, Vaibhav wrote: Hi Loic, On Fri, Dec 21, 2012 at 16:23:24, Loic PALLARDY wrote: On 12/21/2012 11:49 AM, Bedia, Vaibhav wrote: On Fri, Dec 21, 2012 at 14:24:26, Loic PALLARDY wrote: I have a few patches which are dependent on this patch series. Could you please keep me in cc for the future versions. Sure, I'll. When do you plan to post an updated version of these patches? I'm synchronizing with Omar to include TI RPMsg and tidspbridge patches in next update. So I plan update for end of the week, beginning of next week. Ok. Thanks. Regards, Vaibhav -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/9] mailbox: OMAP: introduce mailbox framework
On Fri, Dec 21, 2012 at 14:24:26, Loic PALLARDY wrote: > > > On 12/18/2012 05:59 PM, Tony Lindgren wrote: > > * Loic Pallardy [121218 05:15]: > >> > >> Signed-off-by: Omar Ramirez Luna > > > > AFAIK the first two patches should have: > > From: Omar Ramirez Luna > Yes right, my mistake. > I'll fix that on next version. > /Loic I have a few patches which are dependent on this patch series. Could you please keep me in cc for the future versions. Thanks, Vaibhav -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 4/9] mailbox: create opened message type
On Tue, Dec 18, 2012 at 18:40:07, Loic Pallardy wrote:
> Current message type is a u32 to fit HW fifo format.
> This should be extended to support any message exchanges
> and type of mailbox.
> Propose structure owns the original u32 and an optional
> pointer on additional data.
>
> Signed-off-by: Loic Pallardy
> ---
> drivers/mailbox/Kconfig | 9 ++
> drivers/mailbox/mailbox-omap1.c | 18 ++--
> drivers/mailbox/mailbox-omap2.c | 10 ---
> drivers/mailbox/mailbox.c | 64
> +
> drivers/mailbox/mailbox.h | 6 ++--
> include/linux/mailbox.h | 17 ++-
> 6 files changed, 89 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
> index d1e7d74..efb766f 100644
> --- a/drivers/mailbox/Kconfig
> +++ b/drivers/mailbox/Kconfig
> @@ -33,4 +33,13 @@ config MBOX_KFIFO_SIZE
> This can also be changed at runtime (via the mbox_kfifo_size
> module parameter).
>
> +config MBOX_DATA_SIZE
> + int "Mailbox associated data max size (bytes)"
> + default 64
> + help
> + Specify the default size of mailbox's associated data buffer
> + (bytes)
> + This can also be changed at runtime (via the mbox_kfifo_size
> + module parameter).
> +
> endif
> diff --git a/drivers/mailbox/mailbox-omap1.c b/drivers/mailbox/mailbox-omap1.c
> index 31cb70a..94e90af 100644
> --- a/drivers/mailbox/mailbox-omap1.c
> +++ b/drivers/mailbox/mailbox-omap1.c
> @@ -50,26 +50,26 @@ static inline void mbox_write_reg(u32 val, size_t ofs)
> }
>
> /* msg */
> -static mbox_msg_t omap1_mbox_fifo_read(struct mailbox *mbox)
> +static int omap1_mbox_fifo_read(struct mailbox *mbox, struct mailbox_msg
> *msg)
> {
> struct omap_mbox1_fifo *fifo =
> &((struct omap_mbox1_priv *)mbox->priv)->rx_fifo;
> - mbox_msg_t msg;
>
> - msg = mbox_read_reg(fifo->data);
> - msg |= ((mbox_msg_t) mbox_read_reg(fifo->cmd)) << 16;
> + msg->header = mbox_read_reg(fifo->data);
> + msg->header |= ((mbox_msg_t) mbox_read_reg(fifo->cmd)) << 16;
Now that struct mailbox_msg encapsulates the data, you can
get rid of the mbox_msg_t typedef completely. Having the data
as part of the mailbox_msg along with the functions with mbox_msg_t
as the return type just creates confusion IMHO.
>
> - return msg;
> + return 0;
> }
Convert all return 0 functions to void?
[...]
>
> diff --git a/include/linux/mailbox.h b/include/linux/mailbox.h
> index 18c9502..d256e1a 100644
> --- a/include/linux/mailbox.h
> +++ b/include/linux/mailbox.h
> @@ -7,7 +7,22 @@ typedef int __bitwise mailbox_irq_t;
> #define IRQ_TX ((__force mailbox_irq_t) 1)
> #define IRQ_RX ((__force mailbox_irq_t) 2)
>
> -int mailbox_msg_send(struct mailbox *, mbox_msg_t msg);
> +struct mailbox_msg {
> + int size;
> + u32 header;
> + void*pdata;
> +};
> +
> +#define MAILBOX_FILL_MSG(_msg, _header, _pdata, _size) { \
> + _msg.header = _header; \
> + _msg.pdata = (void *)_pdata; \
> + _msg.size = _size; \
> +}
> +
> +#define MAILBOX_FILL_HEADER_MSG(_msg, _header) \
> + MAILBOX_FILL_MSG(_msg, _header, NULL, 0);
> +
I used these patches as part of the suspend-resume support for AM335x
which has the same mailbox IP as OMAP4. I used the MAILBOX_FILL_HEADER_MSG
helper and things work as expected.
However, I found the 'header' part to be very confusing. Why not treat the
OMAP case as a special case of the new MAILBOX_FILL_MSG where the data size
is set to 1?
Regards,
Vaibhav
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 5/9] mailbox: change protection mechanisms
On Tue, Dec 18, 2012 at 18:40:08, Loic Pallardy wrote: > TX: replace spin by mutex > RX: replace spin_lock_irq by spin_lock_irqsave > Can you please add a short note on why this is being done? Regards, Vaibhav -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 5/9] mailbox: change protection mechanisms
On Tue, Dec 18, 2012 at 18:40:08, Loic Pallardy wrote: TX: replace spin by mutex RX: replace spin_lock_irq by spin_lock_irqsave Can you please add a short note on why this is being done? Regards, Vaibhav -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 4/9] mailbox: create opened message type
On Tue, Dec 18, 2012 at 18:40:07, Loic Pallardy wrote:
Current message type is a u32 to fit HW fifo format.
This should be extended to support any message exchanges
and type of mailbox.
Propose structure owns the original u32 and an optional
pointer on additional data.
Signed-off-by: Loic Pallardy loic.palla...@st.com
---
drivers/mailbox/Kconfig | 9 ++
drivers/mailbox/mailbox-omap1.c | 18 ++--
drivers/mailbox/mailbox-omap2.c | 10 ---
drivers/mailbox/mailbox.c | 64
+
drivers/mailbox/mailbox.h | 6 ++--
include/linux/mailbox.h | 17 ++-
6 files changed, 89 insertions(+), 35 deletions(-)
diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index d1e7d74..efb766f 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -33,4 +33,13 @@ config MBOX_KFIFO_SIZE
This can also be changed at runtime (via the mbox_kfifo_size
module parameter).
+config MBOX_DATA_SIZE
+ int Mailbox associated data max size (bytes)
+ default 64
+ help
+ Specify the default size of mailbox's associated data buffer
+ (bytes)
+ This can also be changed at runtime (via the mbox_kfifo_size
+ module parameter).
+
endif
diff --git a/drivers/mailbox/mailbox-omap1.c b/drivers/mailbox/mailbox-omap1.c
index 31cb70a..94e90af 100644
--- a/drivers/mailbox/mailbox-omap1.c
+++ b/drivers/mailbox/mailbox-omap1.c
@@ -50,26 +50,26 @@ static inline void mbox_write_reg(u32 val, size_t ofs)
}
/* msg */
-static mbox_msg_t omap1_mbox_fifo_read(struct mailbox *mbox)
+static int omap1_mbox_fifo_read(struct mailbox *mbox, struct mailbox_msg
*msg)
{
struct omap_mbox1_fifo *fifo =
((struct omap_mbox1_priv *)mbox-priv)-rx_fifo;
- mbox_msg_t msg;
- msg = mbox_read_reg(fifo-data);
- msg |= ((mbox_msg_t) mbox_read_reg(fifo-cmd)) 16;
+ msg-header = mbox_read_reg(fifo-data);
+ msg-header |= ((mbox_msg_t) mbox_read_reg(fifo-cmd)) 16;
Now that struct mailbox_msg encapsulates the data, you can
get rid of the mbox_msg_t typedef completely. Having the data
as part of the mailbox_msg along with the functions with mbox_msg_t
as the return type just creates confusion IMHO.
- return msg;
+ return 0;
}
Convert all return 0 functions to void?
[...]
diff --git a/include/linux/mailbox.h b/include/linux/mailbox.h
index 18c9502..d256e1a 100644
--- a/include/linux/mailbox.h
+++ b/include/linux/mailbox.h
@@ -7,7 +7,22 @@ typedef int __bitwise mailbox_irq_t;
#define IRQ_TX ((__force mailbox_irq_t) 1)
#define IRQ_RX ((__force mailbox_irq_t) 2)
-int mailbox_msg_send(struct mailbox *, mbox_msg_t msg);
+struct mailbox_msg {
+ int size;
+ u32 header;
+ void*pdata;
+};
+
+#define MAILBOX_FILL_MSG(_msg, _header, _pdata, _size) { \
+ _msg.header = _header; \
+ _msg.pdata = (void *)_pdata; \
+ _msg.size = _size; \
+}
+
+#define MAILBOX_FILL_HEADER_MSG(_msg, _header) \
+ MAILBOX_FILL_MSG(_msg, _header, NULL, 0);
+
I used these patches as part of the suspend-resume support for AM335x
which has the same mailbox IP as OMAP4. I used the MAILBOX_FILL_HEADER_MSG
helper and things work as expected.
However, I found the 'header' part to be very confusing. Why not treat the
OMAP case as a special case of the new MAILBOX_FILL_MSG where the data size
is set to 1?
Regards,
Vaibhav
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/9] mailbox: OMAP: introduce mailbox framework
On Fri, Dec 21, 2012 at 14:24:26, Loic PALLARDY wrote: On 12/18/2012 05:59 PM, Tony Lindgren wrote: * Loic Pallardyloic.pallardy-...@stericsson.com [121218 05:15]: Signed-off-by: Omar Ramirez Lunaomar.l...@linaro.org AFAIK the first two patches should have: From: Omar Ramirez Lunaomar.l...@linaro.org Yes right, my mistake. I'll fix that on next version. /Loic I have a few patches which are dependent on this patch series. Could you please keep me in cc for the future versions. Thanks, Vaibhav -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: BUG: spinlock bad magic on CPU#0 on BeagleBone
On Thu, Dec 20, 2012 at 11:55:24, Stephen Boyd wrote: > On 12/19/2012 8:48 PM, Bedia, Vaibhav wrote: > > I tried out 3 variants of AM335x boards - 2 of these (BeagleBone and EVM) > > have DDR2 > > and 1 has DDR3 (EVM-SK). The BUG is triggered on all of these at the same > > point. > > > > With Stephen's change I don't see this on any of the board variants :) > > New bootlog below. > > Great! Can I have your Tested-by then? I'll wrap it up into a patch. Is > this is a new regression? From a glance at the code it looks to have > existed for quite a while now. I went back to a branch based off 3.7-rc4 and don't see the issue there. Not sure what is triggering this now. Tested-by: Vaibhav Bedia -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: BUG: spinlock bad magic on CPU#0 on BeagleBone
On Thu, Dec 20, 2012 at 01:53:42, Stephen Boyd wrote:
> On 12/19/12 08:53, Paul Walmsley wrote:
> > On Wed, 19 Dec 2012, Bedia, Vaibhav wrote:
> >
> >> Current mainline on Beaglebone using the omap2plus_defconfig + 3 build
> >> fixes
> >> is triggering a BUG()
> > ...
> >
> >> [0.109688] Security Framework initialized
> >> [0.109889] Mount-cache hash table entries: 512
> >> [0.112674] BUG: spinlock bad magic on CPU#0, swapper/0/0
> >> [0.112724] lock: atomic64_lock+0x240/0x400, .magic: , .owner:
> >> /-1, .owner_cpu: 0
> >> [0.112782] [] (unwind_backtrace+0x0/0xf0) from []
> >> (do_raw_spin_lock+0x158/0x198)
> >> [0.112813] [] (do_raw_spin_lock+0x158/0x198) from
> >> [] (_raw_spin_lock_irqsave+0x4c/0x58)
> >> [0.112844] [] (_raw_spin_lock_irqsave+0x4c/0x58) from
> >> [] (atomic64_add_return+0x30/0x5c)
> >> [0.112886] [] (atomic64_add_return+0x30/0x5c) from
> >> [] (alloc_mnt_ns.clone.14+0x44/0xac)
> >> [0.112914] [] (alloc_mnt_ns.clone.14+0x44/0xac) from
> >> [] (create_mnt_ns+0xc/0x54)
> >> [0.112951] [] (create_mnt_ns+0xc/0x54) from []
> >> (mnt_init+0x120/0x1d4)
> >> [0.112978] [] (mnt_init+0x120/0x1d4) from []
> >> (vfs_caches_init+0xe0/0x10c)
> >> [0.113005] [] (vfs_caches_init+0xe0/0x10c) from []
> >> (start_kernel+0x29c/0x300)
> >> [0.113029] [] (start_kernel+0x29c/0x300) from [<80008078>]
> >> (0x80008078)
> >> [0.118290] CPU: Testing write buffer coherency: ok
> >> [0.118968] CPU0: thread -1, cpu 0, socket -1, mpidr 0
> >> [0.119053] Setting up static identity map for 0x804de2c8 - 0x804de338
> >> [0.120698] Brought up 1 CPUs
> > This is probably a memory corruption bug, there's probably some code
> > executing early that's writing outside its own data and trashing some
> > previously-allocated memory.
>
> I'm not so sure. It looks like atomic64s use spinlocks on processors
> that don't have 64-bit atomic instructions (see lib/atomic64.c). And
> those spinlocks are not initialized until a pure initcall runs,
> init_atomic64_lock(). Pure initcalls don't run until after
> vfs_caches_init() and so you get this BUG() warning that the spinlock is
> not initialized.
>
> How about we initialize the locks statically? Does that fix your problem?
>
> >8-
>
> diff --git a/lib/atomic64.c b/lib/atomic64.c
> index 9785378..08a4f06 100644
> --- a/lib/atomic64.c
> +++ b/lib/atomic64.c
> @@ -31,7 +31,11 @@
> static union {
> raw_spinlock_t lock;
> char pad[L1_CACHE_BYTES];
> -} atomic64_lock[NR_LOCKS] __cacheline_aligned_in_smp;
> +} atomic64_lock[NR_LOCKS] __cacheline_aligned_in_smp = {
> + [0 ... (NR_LOCKS - 1)] = {
> + .lock = __RAW_SPIN_LOCK_UNLOCKED(atomic64_lock.lock),
> + },
> +};
>
> static inline raw_spinlock_t *lock_addr(const atomic64_t *v)
> {
> @@ -173,14 +177,3 @@ int atomic64_add_unless(atomic64_t *v, long long a, long
> long u)
> return ret;
> }
> EXPORT_SYMBOL(atomic64_add_unless);
> -
> -static int init_atomic64_lock(void)
> -{
> - int i;
> -
> - for (i = 0; i < NR_LOCKS; ++i)
> - raw_spin_lock_init(_lock[i].lock);
> - return 0;
> -}
> -
> -pure_initcall(init_atomic64_lock);
>
I tried out 3 variants of AM335x boards - 2 of these (BeagleBone and EVM) have
DDR2
and 1 has DDR3 (EVM-SK). The BUG is triggered on all of these at the same point.
With Stephen's change I don't see this on any of the board variants :)
New bootlog below.
Thanks,
Vaibhav
---
[0.00] Booting Linux on physical CPU 0x0
[0.00] Linux version 3.7.0-01415-g55bc169-dirty (a0393953@psplinux063)
(gcc version 4.5.3 20110311 (prerelease) (GCC) ) #4 SMP Thu Dec 20 09:59:12 IST
2012
[0.00] CPU: ARMv7 Processor [413fc082] revision 2 (ARMv7), cr=10c53c7d
[0.00] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing
instruction cache
[0.00] Machine: Generic AM33XX (Flattened Device Tree), model: TI
AM335x BeagleBone
[0.00] Memory policy: ECC disabled, Data cache writeback
[0.00] AM335X ES1.0 (neon )
[0.00] PERCPU: Embedded 9 pages/cpu @c0f1a000 s12992 r8192 d15680 u36864
[0.00] Built 1 zonelists in Zone order, mobility grouping on. Total
pages: 64768
[0.00] Kernel command line: console=ttyO0,115200n8 mem=256M
root=/dev/ram rw initrd=0x8200,16MB ramdisk_size=65536 earlyprintk=serial
[0.00] PID hash table entries: 1024 (order: 0, 4096 bytes)
[0.00] Dentry cache hash table en
RE: BUG: spinlock bad magic on CPU#0 on BeagleBone
On Thu, Dec 20, 2012 at 01:53:42, Stephen Boyd wrote:
On 12/19/12 08:53, Paul Walmsley wrote:
On Wed, 19 Dec 2012, Bedia, Vaibhav wrote:
Current mainline on Beaglebone using the omap2plus_defconfig + 3 build
fixes
is triggering a BUG()
...
[0.109688] Security Framework initialized
[0.109889] Mount-cache hash table entries: 512
[0.112674] BUG: spinlock bad magic on CPU#0, swapper/0/0
[0.112724] lock: atomic64_lock+0x240/0x400, .magic: , .owner:
none/-1, .owner_cpu: 0
[0.112782] [c001af64] (unwind_backtrace+0x0/0xf0) from [c02c2010]
(do_raw_spin_lock+0x158/0x198)
[0.112813] [c02c2010] (do_raw_spin_lock+0x158/0x198) from
[c04d89ec] (_raw_spin_lock_irqsave+0x4c/0x58)
[0.112844] [c04d89ec] (_raw_spin_lock_irqsave+0x4c/0x58) from
[c02cabf0] (atomic64_add_return+0x30/0x5c)
[0.112886] [c02cabf0] (atomic64_add_return+0x30/0x5c) from
[c0124564] (alloc_mnt_ns.clone.14+0x44/0xac)
[0.112914] [c0124564] (alloc_mnt_ns.clone.14+0x44/0xac) from
[c0124f4c] (create_mnt_ns+0xc/0x54)
[0.112951] [c0124f4c] (create_mnt_ns+0xc/0x54) from [c06f31a4]
(mnt_init+0x120/0x1d4)
[0.112978] [c06f31a4] (mnt_init+0x120/0x1d4) from [c06f2d50]
(vfs_caches_init+0xe0/0x10c)
[0.113005] [c06f2d50] (vfs_caches_init+0xe0/0x10c) from [c06d4798]
(start_kernel+0x29c/0x300)
[0.113029] [c06d4798] (start_kernel+0x29c/0x300) from [80008078]
(0x80008078)
[0.118290] CPU: Testing write buffer coherency: ok
[0.118968] CPU0: thread -1, cpu 0, socket -1, mpidr 0
[0.119053] Setting up static identity map for 0x804de2c8 - 0x804de338
[0.120698] Brought up 1 CPUs
This is probably a memory corruption bug, there's probably some code
executing early that's writing outside its own data and trashing some
previously-allocated memory.
I'm not so sure. It looks like atomic64s use spinlocks on processors
that don't have 64-bit atomic instructions (see lib/atomic64.c). And
those spinlocks are not initialized until a pure initcall runs,
init_atomic64_lock(). Pure initcalls don't run until after
vfs_caches_init() and so you get this BUG() warning that the spinlock is
not initialized.
How about we initialize the locks statically? Does that fix your problem?
8-
diff --git a/lib/atomic64.c b/lib/atomic64.c
index 9785378..08a4f06 100644
--- a/lib/atomic64.c
+++ b/lib/atomic64.c
@@ -31,7 +31,11 @@
static union {
raw_spinlock_t lock;
char pad[L1_CACHE_BYTES];
-} atomic64_lock[NR_LOCKS] __cacheline_aligned_in_smp;
+} atomic64_lock[NR_LOCKS] __cacheline_aligned_in_smp = {
+ [0 ... (NR_LOCKS - 1)] = {
+ .lock = __RAW_SPIN_LOCK_UNLOCKED(atomic64_lock.lock),
+ },
+};
static inline raw_spinlock_t *lock_addr(const atomic64_t *v)
{
@@ -173,14 +177,3 @@ int atomic64_add_unless(atomic64_t *v, long long a, long
long u)
return ret;
}
EXPORT_SYMBOL(atomic64_add_unless);
-
-static int init_atomic64_lock(void)
-{
- int i;
-
- for (i = 0; i NR_LOCKS; ++i)
- raw_spin_lock_init(atomic64_lock[i].lock);
- return 0;
-}
-
-pure_initcall(init_atomic64_lock);
I tried out 3 variants of AM335x boards - 2 of these (BeagleBone and EVM) have
DDR2
and 1 has DDR3 (EVM-SK). The BUG is triggered on all of these at the same point.
With Stephen's change I don't see this on any of the board variants :)
New bootlog below.
Thanks,
Vaibhav
---
[0.00] Booting Linux on physical CPU 0x0
[0.00] Linux version 3.7.0-01415-g55bc169-dirty (a0393953@psplinux063)
(gcc version 4.5.3 20110311 (prerelease) (GCC) ) #4 SMP Thu Dec 20 09:59:12 IST
2012
[0.00] CPU: ARMv7 Processor [413fc082] revision 2 (ARMv7), cr=10c53c7d
[0.00] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing
instruction cache
[0.00] Machine: Generic AM33XX (Flattened Device Tree), model: TI
AM335x BeagleBone
[0.00] Memory policy: ECC disabled, Data cache writeback
[0.00] AM335X ES1.0 (neon )
[0.00] PERCPU: Embedded 9 pages/cpu @c0f1a000 s12992 r8192 d15680 u36864
[0.00] Built 1 zonelists in Zone order, mobility grouping on. Total
pages: 64768
[0.00] Kernel command line: console=ttyO0,115200n8 mem=256M
root=/dev/ram rw initrd=0x8200,16MB ramdisk_size=65536 earlyprintk=serial
[0.00] PID hash table entries: 1024 (order: 0, 4096 bytes)
[0.00] Dentry cache hash table entries: 32768 (order: 5, 131072 bytes)
[0.00] Inode-cache hash table entries: 16384 (order: 4, 65536 bytes)
[0.00] __ex_table already sorted, skipping sort
[0.00] Memory: 255MB = 255MB total
[0.00] Memory: 229012k/229012k available, 33132k reserved, 0K highmem
[0.00] Virtual kernel memory layout:
[0.00] vector : 0x - 0x1000 ( 4 kB)
[0.00] fixmap : 0xfff0 - 0xfffe ( 896 kB)
[0.00
RE: BUG: spinlock bad magic on CPU#0 on BeagleBone
On Thu, Dec 20, 2012 at 11:55:24, Stephen Boyd wrote: On 12/19/2012 8:48 PM, Bedia, Vaibhav wrote: I tried out 3 variants of AM335x boards - 2 of these (BeagleBone and EVM) have DDR2 and 1 has DDR3 (EVM-SK). The BUG is triggered on all of these at the same point. With Stephen's change I don't see this on any of the board variants :) New bootlog below. Great! Can I have your Tested-by then? I'll wrap it up into a patch. Is this is a new regression? From a glance at the code it looks to have existed for quite a while now. I went back to a branch based off 3.7-rc4 and don't see the issue there. Not sure what is triggering this now. Tested-by: Vaibhav Bedia vaibhav.be...@ti.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: arch/arm/mach-omap2/i2c.c:130:2: error: implicit declaration of function 'omap_pm_set_max_mpu_wakeup_lat'
On Wed, Dec 19, 2012 at 07:52:47, Fengguang Wu wrote: > [add more CC] > > On Wed, Dec 19, 2012 at 10:11:02AM +0800, Fengguang Wu wrote: > > Hi Linus, Wolfram, > > > > FYI, kernel build failed on > > > > tree: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux master > > head: 752451f01c4567b506bf4343082682dbb8fb30dd > > commit: 752451f01c4567b506bf4343082682dbb8fb30dd Merge branch > > 'i2c-embedded/for-next' of git://git.pengutronix.de/git/wsa/linux > > date: 56 minutes ago > > config: make ARCH=arm omap2plus_defconfig > > > > All error/warnings: > > > > arch/arm/mach-omap2/i2c.c: In function > > 'omap_pm_set_max_mpu_wakeup_lat_compat': > > arch/arm/mach-omap2/i2c.c:130:2: error: implicit declaration of function > > 'omap_pm_set_max_mpu_wakeup_lat' [-Werror=implicit-function-declaration] > > cc1: some warnings being treated as errors > > > > The root cause is, i2c.c (which is always compiled in > > arch/arm/mach-omap2/Makefile) > > uses omap_pm_set_max_mpu_wakeup_lat() which is defined in omap-pm-noop.c > > (which > > is compiled only on CONFIG_OMAP_PM_NOOP). CONFIG_OMAP_PM_NOOP is defined. The build breakage is due to a missing header inclusion. I just sent a patch for this [1] There are still a couple of patches missing before omap2plus_defconfig compiles https://patchwork.kernel.org/patch/1810481/ https://patchwork.kernel.org/patch/1810441/ I guess they are on the way from some other tree. Regards, Vaibhav [1] http://marc.info/?l=linux-arm-kernel=135589966017628=2 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: arch/arm/mach-omap2/i2c.c:130:2: error: implicit declaration of function 'omap_pm_set_max_mpu_wakeup_lat'
On Wed, Dec 19, 2012 at 07:52:47, Fengguang Wu wrote: [add more CC] On Wed, Dec 19, 2012 at 10:11:02AM +0800, Fengguang Wu wrote: Hi Linus, Wolfram, FYI, kernel build failed on tree: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux master head: 752451f01c4567b506bf4343082682dbb8fb30dd commit: 752451f01c4567b506bf4343082682dbb8fb30dd Merge branch 'i2c-embedded/for-next' of git://git.pengutronix.de/git/wsa/linux date: 56 minutes ago config: make ARCH=arm omap2plus_defconfig All error/warnings: arch/arm/mach-omap2/i2c.c: In function 'omap_pm_set_max_mpu_wakeup_lat_compat': arch/arm/mach-omap2/i2c.c:130:2: error: implicit declaration of function 'omap_pm_set_max_mpu_wakeup_lat' [-Werror=implicit-function-declaration] cc1: some warnings being treated as errors The root cause is, i2c.c (which is always compiled in arch/arm/mach-omap2/Makefile) uses omap_pm_set_max_mpu_wakeup_lat() which is defined in omap-pm-noop.c (which is compiled only on CONFIG_OMAP_PM_NOOP). CONFIG_OMAP_PM_NOOP is defined. The build breakage is due to a missing header inclusion. I just sent a patch for this [1] There are still a couple of patches missing before omap2plus_defconfig compiles https://patchwork.kernel.org/patch/1810481/ https://patchwork.kernel.org/patch/1810441/ I guess they are on the way from some other tree. Regards, Vaibhav [1] http://marc.info/?l=linux-arm-kernelm=135589966017628w=2 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v3 03/10] ARM: OMAP: AM33xx hwmod: Add parent-child relationship for PWM subsystem
Hi Benoit, On Mon, Nov 26, 2012 at 14:32:59, Cousson, Benoit wrote: > Hi Vaibhav, > > On 11/26/2012 06:19 AM, Bedia, Vaibhav wrote: > > On Fri, Nov 23, 2012 at 16:36:06, Philip, Avinash wrote: > >> On Tue, Nov 20, 2012 at 10:33:44, Philip, Avinash wrote: > >>> As part of PWM subsystem integration, PWM subsystem are sharing > >>> resources like clock across submodules (ECAP, EQEP & EHRPWM). > >>> To handle resource sharing & IP integration > >>> 1. Rework on parent child relation between PWMSS and > >>>ECAP, EQEP & EHRPWM child devices to support runtime PM. > >>> 2. Add support for opt_clks in EHRPWM HWMOD entry to handle additional > >>>clock gating from control module. > >>> 3. Add HWMOD entries for EQEP PWM submodule. > >>> > >> > >> Is there any review on this patch? > >> This patch depends on ECAP & EHRPWM to work in am335x. > > > > First of all, I think you should break up this patch as per the 3 points > > that you mentioned above. > > > > The usage of opt_clks for this does not look right to me. Based on your > > description this clock is necessary and not optional on AM335x and on > > Davinci platforms this clock does not exist. I checked the DA830 TRM and looks like TBCLK for eHRPWM is an always on clock there. So, the only difference in AM335x is an additional enable bit. Instead of adding this as opt_clk in hwmod, we could add an always on clock node in Davinci clock data and have the driver always do a clk_enable() on the tbclk as part of the probe sequence. On AM335x, with the right clock node this will enable the clock in hardware and on DA830 it turns into a NOP. This way we can avoid adding the opt_clk entry in hwmod of eHRPWM. > > > > I think the custom activate/deactivate functions in the OMAP runtime PM > > implementation was a good fit for keeping this SoC integration detail out > > of the driver code. However, the current DT flow in omap_device.c seems to > > assign the default activate/deactivate ops. Is that approach deprecated? > > The issue is that this approach is not doable anymore with DT, that's > why I had to provide a default set of functions. > So once all OMAP drivers get converted to DT, will there be no notion of latency based activate/deactivate functions? Or will it get used in a different manner? Regards, Vaibhav -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v3 03/10] ARM: OMAP: AM33xx hwmod: Add parent-child relationship for PWM subsystem
Hi Benoit, On Mon, Nov 26, 2012 at 14:32:59, Cousson, Benoit wrote: Hi Vaibhav, On 11/26/2012 06:19 AM, Bedia, Vaibhav wrote: On Fri, Nov 23, 2012 at 16:36:06, Philip, Avinash wrote: On Tue, Nov 20, 2012 at 10:33:44, Philip, Avinash wrote: As part of PWM subsystem integration, PWM subsystem are sharing resources like clock across submodules (ECAP, EQEP EHRPWM). To handle resource sharing IP integration 1. Rework on parent child relation between PWMSS and ECAP, EQEP EHRPWM child devices to support runtime PM. 2. Add support for opt_clks in EHRPWM HWMOD entry to handle additional clock gating from control module. 3. Add HWMOD entries for EQEP PWM submodule. Is there any review on this patch? This patch depends on ECAP EHRPWM to work in am335x. First of all, I think you should break up this patch as per the 3 points that you mentioned above. The usage of opt_clks for this does not look right to me. Based on your description this clock is necessary and not optional on AM335x and on Davinci platforms this clock does not exist. I checked the DA830 TRM and looks like TBCLK for eHRPWM is an always on clock there. So, the only difference in AM335x is an additional enable bit. Instead of adding this as opt_clk in hwmod, we could add an always on clock node in Davinci clock data and have the driver always do a clk_enable() on the tbclk as part of the probe sequence. On AM335x, with the right clock node this will enable the clock in hardware and on DA830 it turns into a NOP. This way we can avoid adding the opt_clk entry in hwmod of eHRPWM. I think the custom activate/deactivate functions in the OMAP runtime PM implementation was a good fit for keeping this SoC integration detail out of the driver code. However, the current DT flow in omap_device.c seems to assign the default activate/deactivate ops. Is that approach deprecated? The issue is that this approach is not doable anymore with DT, that's why I had to provide a default set of functions. So once all OMAP drivers get converted to DT, will there be no notion of latency based activate/deactivate functions? Or will it get used in a different manner? Regards, Vaibhav -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v3 03/10] ARM: OMAP: AM33xx hwmod: Add parent-child relationship for PWM subsystem
On Fri, Nov 23, 2012 at 16:36:06, Philip, Avinash wrote: > On Tue, Nov 20, 2012 at 10:33:44, Philip, Avinash wrote: > > As part of PWM subsystem integration, PWM subsystem are sharing > > resources like clock across submodules (ECAP, EQEP & EHRPWM). > > To handle resource sharing & IP integration > > 1. Rework on parent child relation between PWMSS and > >ECAP, EQEP & EHRPWM child devices to support runtime PM. > > 2. Add support for opt_clks in EHRPWM HWMOD entry to handle additional > >clock gating from control module. > > 3. Add HWMOD entries for EQEP PWM submodule. > > > > Is there any review on this patch? > This patch depends on ECAP & EHRPWM to work in am335x. First of all, I think you should break up this patch as per the 3 points that you mentioned above. The usage of opt_clks for this does not look right to me. Based on your description this clock is necessary and not optional on AM335x and on Davinci platforms this clock does not exist. I think the custom activate/deactivate functions in the OMAP runtime PM implementation was a good fit for keeping this SoC integration detail out of the driver code. However, the current DT flow in omap_device.c seems to assign the default activate/deactivate ops. Is that approach deprecated? Regards, Vaibhav -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v3 03/10] ARM: OMAP: AM33xx hwmod: Add parent-child relationship for PWM subsystem
On Fri, Nov 23, 2012 at 16:36:06, Philip, Avinash wrote: On Tue, Nov 20, 2012 at 10:33:44, Philip, Avinash wrote: As part of PWM subsystem integration, PWM subsystem are sharing resources like clock across submodules (ECAP, EQEP EHRPWM). To handle resource sharing IP integration 1. Rework on parent child relation between PWMSS and ECAP, EQEP EHRPWM child devices to support runtime PM. 2. Add support for opt_clks in EHRPWM HWMOD entry to handle additional clock gating from control module. 3. Add HWMOD entries for EQEP PWM submodule. Is there any review on this patch? This patch depends on ECAP EHRPWM to work in am335x. First of all, I think you should break up this patch as per the 3 points that you mentioned above. The usage of opt_clks for this does not look right to me. Based on your description this clock is necessary and not optional on AM335x and on Davinci platforms this clock does not exist. I think the custom activate/deactivate functions in the OMAP runtime PM implementation was a good fit for keeping this SoC integration detail out of the driver code. However, the current DT flow in omap_device.c seems to assign the default activate/deactivate ops. Is that approach deprecated? Regards, Vaibhav -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 3/8] ARM: OMAP: AM33xx hwmod: Add parent-child relationship for PWM subsystem
On Mon, Nov 05, 2012 at 14:42:24, Philip, Avinash wrote:
[...]
> +
> +static struct omap_hwmod_ocp_if am33xx_epwmss0__ecap0 = {
> + .master = _epwmss0_hwmod,
> + .slave = _ecap0_hwmod,
> + .clk= "l4ls_gclk",
> + .addr = am33xx_ecap0_addr_space,
> + .user = OCP_USER_MPU,
> +};
Noticed this in another patch which is quite similar so commenting here
again. Is the user field required if you are just creating a parent-child
relationship here?
Regards,
Vaibhav
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 3/8] ARM: OMAP: AM33xx hwmod: Add parent-child relationship for PWM subsystem
On Mon, Nov 05, 2012 at 14:42:24, Philip, Avinash wrote:
[...]
+
+static struct omap_hwmod_ocp_if am33xx_epwmss0__ecap0 = {
+ .master = am33xx_epwmss0_hwmod,
+ .slave = am33xx_ecap0_hwmod,
+ .clk= l4ls_gclk,
+ .addr = am33xx_ecap0_addr_space,
+ .user = OCP_USER_MPU,
+};
Noticed this in another patch which is quite similar so commenting here
again. Is the user field required if you are just creating a parent-child
relationship here?
Regards,
Vaibhav
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 8/8] ARM: dts: AM33XX: Add PWM backlight DT data to am335x-evm
On Mon, Nov 05, 2012 at 14:42:29, Philip, Avinash wrote:
[...]
> + am33xx_pinmux: pinmux@44e10800 {
> + ecap0_pins: backlight_pins {
> + pinctrl-single,pins = <
> + 0x164 0x0 /*
> eCAP0_in_PWM0_out.eCAP0_in_PWM0_out MODE0 */
> + >;
> + };
> + };
> +
Ok I see the pinctrl entries here. Shouldn't this be done before
the change in the driver?
Regards.
Vaibhav
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 5/8] pwm: pwm-tiehrpwm: Add device-tree binding support for EHRPWM driver
On Mon, Nov 05, 2012 at 14:42:26, Philip, Avinash wrote:
[...]
> +#include
> +#include
Pinctrl changes should be separate patch. Morevoer, you don't mention
that you making this change.
> +
> +#include "tipwmss.h"
>
> /* EHRPWM registers and bits definitions */
>
> @@ -107,6 +111,10 @@
> #define AQCSFRC_CSFA_FRCHIGH BIT(1)
> #define AQCSFRC_CSFA_DISSWFRC(BIT(1) | BIT(0))
>
> +#define EPWMCLK_EN_SHIFT 8
> +
> +#define PWM_CELL_SIZE3
> +
> #define NUM_PWM_CHANNEL 2 /* EHRPWM channels */
>
> struct ehrpwm_pwm_chip {
> @@ -392,12 +400,27 @@ static const struct pwm_ops ehrpwm_pwm_ops = {
> .owner = THIS_MODULE,
> };
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id ehrpwm_of_match[] = {
> + {
> + .compatible = "ti,am33xx-ehrpwm",
> + },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, ehrpwm_of_match);
> +#endif
> +
> static int __devinit ehrpwm_pwm_probe(struct platform_device *pdev)
> {
> int ret;
> struct resource *r;
> struct clk *clk;
> struct ehrpwm_pwm_chip *pc;
> + struct pinctrl *pinctrl;
> +
> + pinctrl = devm_pinctrl_get_select_default(>dev);
I didn't see a patch adding the pinctrl entries.
> + if (IS_ERR(pinctrl))
> + dev_warn(>dev, "failed to configure pins from driver\n");
>
> pc = devm_kzalloc(>dev, sizeof(*pc), GFP_KERNEL);
> if (!pc) {
> @@ -419,6 +442,7 @@ static int __devinit ehrpwm_pwm_probe(struct
> platform_device *pdev)
>
> pc->chip.dev = >dev;
> pc->chip.ops = _pwm_ops;
> + pc->chip.of_pwm_n_cells = PWM_CELL_SIZE;
> pc->chip.base = -1;
> pc->chip.npwm = NUM_PWM_CHANNEL;
>
> @@ -437,8 +461,11 @@ static int __devinit ehrpwm_pwm_probe(struct
> platform_device *pdev)
> dev_err(>dev, "pwmchip_add() failed: %d\n", ret);
> return ret;
> }
> -
> pm_runtime_enable(>dev);
> + pm_runtime_get_sync(>dev);
> + pwmss_submodule_state_change(pdev->dev.parent, EPWMCLK_EN_SHIFT, true);
I think you should modify this API to return the status for drivers to check.
Another related question, why should this clock be enabled in probe and not
only when it
is required? Shouldn't it be disabled in suspend?
Regards,
Vaibhav
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 6/8] pwm: pwm-tiehrpwm: Adding TBCLK gating support.
On Mon, Nov 05, 2012 at 14:42:27, Philip, Avinash wrote:
[...]
> + /* Some platforms require explicit tbclk gating */
> + if (of_property_read_bool(pdev->dev.of_node, "tbclkgating")) {
> + pc->tbclk = clk_get(>dev, "tbclk");
> + if (IS_ERR(pc->tbclk)) {
> + dev_err(>dev, "Could not get EHRPWM TBCLK\n");
> + return PTR_ERR(pc->tbclk);
> + }
> + }
> +
> + /* Enable tbclk & leave */
> + if (pc->tbclk)
> + clk_enable(pc->tbclk);
> +
Here also why are you leaving this clock always running?
Regards,
Vaibhav
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/8] PWMSS: Add PWM Subsystem driver for parent<->child relationship
On Mon, Nov 05, 2012 at 14:42:22, Philip, Avinash wrote:
[...]
> +pwmss0: pwmss@4830 {
> + compatible = "ti,am33xx-pwmss";
> + reg = <0x4830 0x10
> + 0x48300100 0x80
> + 0x48300180 0x80
> + 0x48300200 0x80>;
Do you really need the 4 address ranges here? You eventually do add in
child nodes with other address ranges so isn't the first entry sufficient?
I haven't really looked at the DT details so it is enforced by that let me
know.
[...]
> +
> +#define PWMSS_CLKCONFIG 8
> +
This #def can use a comment.
> +void pwmss_submodule_state_change(struct device *dev, int pos, bool enable)
> +{
> + struct pwmss_info *info = dev_get_drvdata(dev);
> + u16 val;
> +
> + val = readw(info->mmio_base + PWMSS_CLKCONFIG);
> + if (enable)
> + val |= 1 << pos;
> + else
> + val &= ~(1 << pos);
> + mutex_lock(>pwmss_lock);
> + writew(val , info->mmio_base + PWMSS_CLKCONFIG);
> + mutex_unlock(>pwmss_lock);
> +}
> +EXPORT_SYMBOL(pwmss_submodule_state_change);
I see a clk_en_ack field in the clock status register. You should be checking
that.
[...]
> +
> +MODULE_DESCRIPTION("pwmss driver");
> +MODULE_AUTHOR("Texas Instruments");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/pwm/tipwmss.h b/drivers/pwm/tipwmss.h
> new file mode 100644
> index 000..83fdc29
> --- /dev/null
> +++ b/drivers/pwm/tipwmss.h
> @@ -0,0 +1,8 @@
License text?
> +#ifdef CONFIG_PWM_TIPWMSS
> +extern void pwmss_submodule_state_change(struct device *dev, int pos,
> + bool enable);
> +#else
> +static inline void pwmss_submodule_state_change(struct device *dev, int pos,
> + bool enable)
> +{}
> +#endif
Regards,
Vaibhav
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/8] PWMSS: Add PWM Subsystem driver for parent-child relationship
On Mon, Nov 05, 2012 at 14:42:22, Philip, Avinash wrote:
[...]
+pwmss0: pwmss@4830 {
+ compatible = ti,am33xx-pwmss;
+ reg = 0x4830 0x10
+ 0x48300100 0x80
+ 0x48300180 0x80
+ 0x48300200 0x80;
Do you really need the 4 address ranges here? You eventually do add in
child nodes with other address ranges so isn't the first entry sufficient?
I haven't really looked at the DT details so it is enforced by that let me
know.
[...]
+
+#define PWMSS_CLKCONFIG 8
+
This #def can use a comment.
+void pwmss_submodule_state_change(struct device *dev, int pos, bool enable)
+{
+ struct pwmss_info *info = dev_get_drvdata(dev);
+ u16 val;
+
+ val = readw(info-mmio_base + PWMSS_CLKCONFIG);
+ if (enable)
+ val |= 1 pos;
+ else
+ val = ~(1 pos);
+ mutex_lock(info-pwmss_lock);
+ writew(val , info-mmio_base + PWMSS_CLKCONFIG);
+ mutex_unlock(info-pwmss_lock);
+}
+EXPORT_SYMBOL(pwmss_submodule_state_change);
I see a clk_en_ack field in the clock status register. You should be checking
that.
[...]
+
+MODULE_DESCRIPTION(pwmss driver);
+MODULE_AUTHOR(Texas Instruments);
+MODULE_LICENSE(GPL);
diff --git a/drivers/pwm/tipwmss.h b/drivers/pwm/tipwmss.h
new file mode 100644
index 000..83fdc29
--- /dev/null
+++ b/drivers/pwm/tipwmss.h
@@ -0,0 +1,8 @@
License text?
+#ifdef CONFIG_PWM_TIPWMSS
+extern void pwmss_submodule_state_change(struct device *dev, int pos,
+ bool enable);
+#else
+static inline void pwmss_submodule_state_change(struct device *dev, int pos,
+ bool enable)
+{}
+#endif
Regards,
Vaibhav
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 6/8] pwm: pwm-tiehrpwm: Adding TBCLK gating support.
On Mon, Nov 05, 2012 at 14:42:27, Philip, Avinash wrote:
[...]
+ /* Some platforms require explicit tbclk gating */
+ if (of_property_read_bool(pdev-dev.of_node, tbclkgating)) {
+ pc-tbclk = clk_get(pdev-dev, tbclk);
+ if (IS_ERR(pc-tbclk)) {
+ dev_err(pdev-dev, Could not get EHRPWM TBCLK\n);
+ return PTR_ERR(pc-tbclk);
+ }
+ }
+
+ /* Enable tbclk leave */
+ if (pc-tbclk)
+ clk_enable(pc-tbclk);
+
Here also why are you leaving this clock always running?
Regards,
Vaibhav
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 5/8] pwm: pwm-tiehrpwm: Add device-tree binding support for EHRPWM driver
On Mon, Nov 05, 2012 at 14:42:26, Philip, Avinash wrote:
[...]
+#include linux/of_device.h
+#include linux/pinctrl/consumer.h
Pinctrl changes should be separate patch. Morevoer, you don't mention
that you making this change.
+
+#include tipwmss.h
/* EHRPWM registers and bits definitions */
@@ -107,6 +111,10 @@
#define AQCSFRC_CSFA_FRCHIGH BIT(1)
#define AQCSFRC_CSFA_DISSWFRC(BIT(1) | BIT(0))
+#define EPWMCLK_EN_SHIFT 8
+
+#define PWM_CELL_SIZE3
+
#define NUM_PWM_CHANNEL 2 /* EHRPWM channels */
struct ehrpwm_pwm_chip {
@@ -392,12 +400,27 @@ static const struct pwm_ops ehrpwm_pwm_ops = {
.owner = THIS_MODULE,
};
+#ifdef CONFIG_OF
+static const struct of_device_id ehrpwm_of_match[] = {
+ {
+ .compatible = ti,am33xx-ehrpwm,
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, ehrpwm_of_match);
+#endif
+
static int __devinit ehrpwm_pwm_probe(struct platform_device *pdev)
{
int ret;
struct resource *r;
struct clk *clk;
struct ehrpwm_pwm_chip *pc;
+ struct pinctrl *pinctrl;
+
+ pinctrl = devm_pinctrl_get_select_default(pdev-dev);
I didn't see a patch adding the pinctrl entries.
+ if (IS_ERR(pinctrl))
+ dev_warn(pdev-dev, failed to configure pins from driver\n);
pc = devm_kzalloc(pdev-dev, sizeof(*pc), GFP_KERNEL);
if (!pc) {
@@ -419,6 +442,7 @@ static int __devinit ehrpwm_pwm_probe(struct
platform_device *pdev)
pc-chip.dev = pdev-dev;
pc-chip.ops = ehrpwm_pwm_ops;
+ pc-chip.of_pwm_n_cells = PWM_CELL_SIZE;
pc-chip.base = -1;
pc-chip.npwm = NUM_PWM_CHANNEL;
@@ -437,8 +461,11 @@ static int __devinit ehrpwm_pwm_probe(struct
platform_device *pdev)
dev_err(pdev-dev, pwmchip_add() failed: %d\n, ret);
return ret;
}
-
pm_runtime_enable(pdev-dev);
+ pm_runtime_get_sync(pdev-dev);
+ pwmss_submodule_state_change(pdev-dev.parent, EPWMCLK_EN_SHIFT, true);
I think you should modify this API to return the status for drivers to check.
Another related question, why should this clock be enabled in probe and not
only when it
is required? Shouldn't it be disabled in suspend?
Regards,
Vaibhav
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 8/8] ARM: dts: AM33XX: Add PWM backlight DT data to am335x-evm
On Mon, Nov 05, 2012 at 14:42:29, Philip, Avinash wrote:
[...]
+ am33xx_pinmux: pinmux@44e10800 {
+ ecap0_pins: backlight_pins {
+ pinctrl-single,pins =
+ 0x164 0x0 /*
eCAP0_in_PWM0_out.eCAP0_in_PWM0_out MODE0 */
+ ;
+ };
+ };
+
Ok I see the pinctrl entries here. Shouldn't this be done before
the change in the driver?
Regards.
Vaibhav
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [RFC PATCH v3 00/16] DMA Engine support for AM33XX
On Fri, Oct 19, 2012 at 22:16:15, Porter, Matt wrote:
> On Fri, Oct 19, 2012 at 12:02:42PM +0000, Bedia, Vaibhav wrote:
> > On Fri, Oct 19, 2012 at 16:45:58, Porter, Matt wrote:
> > > On Fri, Oct 19, 2012 at 10:26:20AM +0000, Bedia, Vaibhav wrote:
> > [...]
> > > >
> > > > I didn't see all the patches that you posted on edma-dmaengine-v3
> > > > but I do seem them on edma-dmaengine-am33xx-v3 branch.
> > >
> > > I see I referenced the wrong branch in the cover letter. Thanks for
> > > testing and noticing this. Sorry to make you hunt for the correct
> > > branch in that repo. ;)
> > >
> >
> > No problem.
> >
> > > https://github.com/ohporter/linux/tree/edma-dmaengine-am33xx-v3
> > > is indeed the correct branch for those wanting to pull this in or
> > > grab some of the not-to-be-merged drivers I used for testing.
> > >
> > > > I added a couple of patches to enable earlyprintk and build the DTB
> > > > appended kernel image uImage-dtb.am335x-evm
> > > >
> > > > Here's what i see
> > > >
> > > > [...]
> > >
> > >
> > >
> > > > [0.175354] edma: probe of 4900.edma failed with error -16
> > >
> > > I missed an uninitialized pdata case in the bug fixes mentioned in
> > > the changelog and the folks previously failing the same way didn't
> > > hit the case I suspect you are hitting. Can you try this and let me
> > > know how it works?
> > >
> >
> > That doesn't help :(
>
> Ok, so I dumped my Linaro toolchain which was masking this issue that
> you got unlucky with on EVM, whereas I was lucky. Switching toolchains I
> was able to reproduce the problem. Pantelis Antoniou suggested some
> changes and the following fixes this issue for me...verified on both
> BeagleBone and EVM. Let me know if that works on your end and I'll
> incorporate some version of it in the next update.
Heh I would not have suspected the toolchain so early ;)
>
> diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
> index b761b7a..6ed394f 100644
> --- a/arch/arm/common/edma.c
> +++ b/arch/arm/common/edma.c
> @@ -1598,6 +1598,8 @@ static struct of_dma_filter_info edma_filter_info = {
> static int __init edma_probe(struct platform_device *pdev)
> {
> struct edma_soc_info**info = pdev->dev.platform_data;
> + struct edma_soc_info*ninfo[EDMA_MAX_CC] = {NULL, NULL};
> + struct edma_soc_infotmpinfo;
> s8 (*queue_priority_mapping)[2];
> s8 (*queue_tc_mapping)[2];
> int i, j, off, ln, found = 0;
> @@ -1614,15 +1616,13 @@ static int __init edma_probe(struct platform_device
> *pdev)
> charirq_name[10];
> struct device_node *node = pdev->dev.of_node;
> struct device *dev = >dev;
> - struct edma_soc_info*pdata;
> int ret;
>
> if (node) {
> - pdata = devm_kzalloc(dev,
> - sizeof(struct edma_soc_info),
> - GFP_KERNEL);
> - edma_of_parse_dt(dev, node, pdata);
> - info =
> + info = ninfo;
> + edma_of_parse_dt(dev, node, );
> + info[0] =
> +
> dma_cap_set(DMA_SLAVE, edma_filter_info.dma_cap);
> of_dma_controller_register(dev->of_node,
> of_dma_simple_xlate,
>
With the above diff, the kernel boots fine on the EVM.
> > Looking at the original crash log, I suspect something is not correct
> > with the irq portion, probably in the DT or the driver.
> >
> > "genirq: Flags mismatch irq 28. (edma) vs. (edma)"
> >
> > The warning below that is coming due to fail case in edma_probe not tracking
> > the request_irq status properly and but IMO that's a separate issue.
>
> It is a separate issue, indeed. My ideal goal was to avoid changing
> anything in this existing davinci dma implementation, that's why the
> error paths were unmodified. Since I'm having to rework a few more things
> I'll look at those and generate an improved version.
>
> Russ Dill also made some good simplification/cleanup suggestions for the
> of parsing on irc which I'll incorporate in the next version.
Ok, sounds good.
Regards,
Vaibhav
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [RFC PATCH v3 00/16] DMA Engine support for AM33XX
On Fri, Oct 19, 2012 at 22:16:15, Porter, Matt wrote:
On Fri, Oct 19, 2012 at 12:02:42PM +, Bedia, Vaibhav wrote:
On Fri, Oct 19, 2012 at 16:45:58, Porter, Matt wrote:
On Fri, Oct 19, 2012 at 10:26:20AM +, Bedia, Vaibhav wrote:
[...]
I didn't see all the patches that you posted on edma-dmaengine-v3
but I do seem them on edma-dmaengine-am33xx-v3 branch.
I see I referenced the wrong branch in the cover letter. Thanks for
testing and noticing this. Sorry to make you hunt for the correct
branch in that repo. ;)
No problem.
https://github.com/ohporter/linux/tree/edma-dmaengine-am33xx-v3
is indeed the correct branch for those wanting to pull this in or
grab some of the not-to-be-merged drivers I used for testing.
I added a couple of patches to enable earlyprintk and build the DTB
appended kernel image uImage-dtb.am335x-evm
Here's what i see
[...]
snip
[0.175354] edma: probe of 4900.edma failed with error -16
I missed an uninitialized pdata case in the bug fixes mentioned in
the changelog and the folks previously failing the same way didn't
hit the case I suspect you are hitting. Can you try this and let me
know how it works?
That doesn't help :(
Ok, so I dumped my Linaro toolchain which was masking this issue that
you got unlucky with on EVM, whereas I was lucky. Switching toolchains I
was able to reproduce the problem. Pantelis Antoniou suggested some
changes and the following fixes this issue for me...verified on both
BeagleBone and EVM. Let me know if that works on your end and I'll
incorporate some version of it in the next update.
Heh I would not have suspected the toolchain so early ;)
diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
index b761b7a..6ed394f 100644
--- a/arch/arm/common/edma.c
+++ b/arch/arm/common/edma.c
@@ -1598,6 +1598,8 @@ static struct of_dma_filter_info edma_filter_info = {
static int __init edma_probe(struct platform_device *pdev)
{
struct edma_soc_info**info = pdev-dev.platform_data;
+ struct edma_soc_info*ninfo[EDMA_MAX_CC] = {NULL, NULL};
+ struct edma_soc_infotmpinfo;
s8 (*queue_priority_mapping)[2];
s8 (*queue_tc_mapping)[2];
int i, j, off, ln, found = 0;
@@ -1614,15 +1616,13 @@ static int __init edma_probe(struct platform_device
*pdev)
charirq_name[10];
struct device_node *node = pdev-dev.of_node;
struct device *dev = pdev-dev;
- struct edma_soc_info*pdata;
int ret;
if (node) {
- pdata = devm_kzalloc(dev,
- sizeof(struct edma_soc_info),
- GFP_KERNEL);
- edma_of_parse_dt(dev, node, pdata);
- info = pdata;
+ info = ninfo;
+ edma_of_parse_dt(dev, node, tmpinfo);
+ info[0] = tmpinfo;
+
dma_cap_set(DMA_SLAVE, edma_filter_info.dma_cap);
of_dma_controller_register(dev-of_node,
of_dma_simple_xlate,
With the above diff, the kernel boots fine on the EVM.
Looking at the original crash log, I suspect something is not correct
with the irq portion, probably in the DT or the driver.
genirq: Flags mismatch irq 28. (edma) vs. (edma)
The warning below that is coming due to fail case in edma_probe not tracking
the request_irq status properly and but IMO that's a separate issue.
It is a separate issue, indeed. My ideal goal was to avoid changing
anything in this existing davinci dma implementation, that's why the
error paths were unmodified. Since I'm having to rework a few more things
I'll look at those and generate an improved version.
Russ Dill also made some good simplification/cleanup suggestions for the
of parsing on irc which I'll incorporate in the next version.
Ok, sounds good.
Regards,
Vaibhav
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [RFC PATCH v3 00/16] DMA Engine support for AM33XX
On Fri, Oct 19, 2012 at 16:45:58, Porter, Matt wrote: > On Fri, Oct 19, 2012 at 10:26:20AM +0000, Bedia, Vaibhav wrote: [...] > > > > I didn't see all the patches that you posted on edma-dmaengine-v3 > > but I do seem them on edma-dmaengine-am33xx-v3 branch. > > I see I referenced the wrong branch in the cover letter. Thanks for > testing and noticing this. Sorry to make you hunt for the correct > branch in that repo. ;) > No problem. > https://github.com/ohporter/linux/tree/edma-dmaengine-am33xx-v3 > is indeed the correct branch for those wanting to pull this in or > grab some of the not-to-be-merged drivers I used for testing. > > > I added a couple of patches to enable earlyprintk and build the DTB > > appended kernel image uImage-dtb.am335x-evm > > > > Here's what i see > > > > [...] > > > > > [0.175354] edma: probe of 4900.edma failed with error -16 > > I missed an uninitialized pdata case in the bug fixes mentioned in > the changelog and the folks previously failing the same way didn't > hit the case I suspect you are hitting. Can you try this and let me > know how it works? > That doesn't help :( Looking at the original crash log, I suspect something is not correct with the irq portion, probably in the DT or the driver. "genirq: Flags mismatch irq 28. (edma) vs. (edma)" The warning below that is coming due to fail case in edma_probe not tracking the request_irq status properly and but IMO that's a separate issue. BTW, I am trying this on the EVM. Regards, Vaibhav -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [RFC PATCH v3 00/16] DMA Engine support for AM33XX
Hi Matt, On Thu, Oct 18, 2012 at 18:56:39, Porter, Matt wrote: > Changes since v2: > - Rebased on 3.7-rc1 > - Fixed bug in DT/pdata parsing first found by Gururaja > that turned out to be masked by some toolchains > - Dropped unused mach-omap2/devices.c hsmmc patch > - Added AM33XX crossbar DMA event mux support > - Added am335x-evm support > > Changes since v1: > - Rebased on top of mainline from 12250d8 > - Dropped the feature removal schedule patch > - Implemented dma_request_slave_channel_compat() and > converted the mmc and spi drivers to use it > - Dropped unneeded #address-cells and #size-cells from > EDMA DT support > - Moved private EDMA header to linux/platform_data/ and > removed some unneeded definitions > - Fixed parsing of optional properties > > TODO: > - Add dmaengine support for per-channel caps so the > hack to set the maximum segments can be replaced with > a query to the dmaengine driver > > This series adds DMA Engine support for AM33xx, which uses > an EDMA DMAC. The EDMA DMAC has been previously supported by only > a private API implementation (much like the situation with OMAP > DMA) found on the DaVinci family of SoCs. > > The series applies on top of 3.7-rc1 and the following patches: > > - GPMC fails to reserve memory fix: > http://www.spinics.net/lists/linux-omap/msg79675.html > - TPS65910 regulator fix: > https://patchwork.kernel.org/patch/1593651/ > - dmaengine DT support from Vinod's dmaengine_dt branch in > git://git.infradead.org/users/vkoul/slave-dma.git since > 027478851791df751176398be02a3b1c5f6aa824 > > The approach taken is similar to how OMAP DMA is being converted to > DMA Engine support. With the functional EDMA private API already > existing in mach-davinci/dma.c, we first move that to an ARM common > area so it can be shared. Adding DT and runtime PM support to the > private EDMA API implementation allows it to run on AM33xx. AM33xx > *only* boots using DT so we leverage Jon's generic DT DMA helpers to > register EDMA DMAC with the of_dma framework and then add support > for calling the dma_request_slave_channel() API to both the mmc > and spi drivers. > > With this series both BeagleBone and the AM335x EVM have working > MMC and SPI support. > > This is tested on BeagleBone with a SPI framebuffer driver and MMC > rootfs. A trivial gpio DMA event misc driver was used to test the > crossbar DMA event support. It is also tested on the AM335x EVM > with the onboard SPI flash and MMC rootfs. The branch at > https://github.com/ohporter/linux/tree/edma-dmaengine-v3 has the > complete series, dependencies, and some test drivers/defconfigs. > I didn't see all the patches that you posted on edma-dmaengine-v3 but I do seem them on edma-dmaengine-am33xx-v3 branch. I added a couple of patches to enable earlyprintk and build the DTB appended kernel image uImage-dtb.am335x-evm Here's what i see [...] [0.128831] regulator-dummy: no parameters [0.130793] NET: Registered protocol family 16 [0.131694] DMA: preallocated 256 KiB pool for atomic coherent allocations [0.133030] omap-gpmc omap-gpmc: GPMC revision 6.0 [0.153136] platform 4900.edma: alias fck already exists [0.153176] platform 4900.edma: alias fck already exists [0.153199] platform 4900.edma: alias fck already exists [0.158184] OMAP GPIO hardware version 0.1 [0.172844] No ATAGs? [0.172868] hw-breakpoint: debug architecture 0x4 unsupported. [0.174282] genirq: Flags mismatch irq 28. (edma) vs. (edma) [0.174536] [ cut here ] [0.174576] WARNING: at kernel/irq/manage.c:1211 __free_irq+0x9c/0x1c0() [0.174586] Trying to free already-free IRQ 28 [0.174596] Modules linked in: [0.174645] [] (unwind_backtrace+0x0/0xf0) from [] (warn_slowpath_common+0x4c/0x64) [0.174668] [] (warn_slowpath_common+0x4c/0x64) from [] (warn_slowpath_fmt+0x30/0x40) [0.174688] [] (warn_slowpath_fmt+0x30/0x40) from [] (__free_irq+0x9c/0x1c0) [0.174708] [] (__free_irq+0x9c/0x1c0) from [] (free_irq+0x4c/0xa8) [0.174739] [] (free_irq+0x4c/0xa8) from [] (edma_probe+0x93c/0xa24) [0.174770] [] (edma_probe+0x93c/0xa24) from [] (platform_drv_probe+0x18/0x1c) [0.174793] [] (platform_drv_probe+0x18/0x1c) from [] (driver_probe_device+0x84/0x224) [0.174814] [] (driver_probe_device+0x84/0x224) from [] (__driver_attach+0x94/0x98) [0.174834] [] (__driver_attach+0x94/0x98) from [] (bus_for_each_dev+0x50/0x7c) [0.174854] [] (bus_for_each_dev+0x50/0x7c) from [] (bus_add_driver+0xa0/0x240) [0.174874] [] (bus_add_driver+0xa0/0x240) from [] (driver_register+0x78/0x144) [0.174896] [] (driver_register+0x78/0x144) from [] (platform_driver_probe+0x18/0x9c) [0.174918] [] (platform_driver_probe+0x18/0x9c) from []
RE: [RFC PATCH v3 00/16] DMA Engine support for AM33XX
Hi Matt, On Thu, Oct 18, 2012 at 18:56:39, Porter, Matt wrote: Changes since v2: - Rebased on 3.7-rc1 - Fixed bug in DT/pdata parsing first found by Gururaja that turned out to be masked by some toolchains - Dropped unused mach-omap2/devices.c hsmmc patch - Added AM33XX crossbar DMA event mux support - Added am335x-evm support Changes since v1: - Rebased on top of mainline from 12250d8 - Dropped the feature removal schedule patch - Implemented dma_request_slave_channel_compat() and converted the mmc and spi drivers to use it - Dropped unneeded #address-cells and #size-cells from EDMA DT support - Moved private EDMA header to linux/platform_data/ and removed some unneeded definitions - Fixed parsing of optional properties TODO: - Add dmaengine support for per-channel caps so the hack to set the maximum segments can be replaced with a query to the dmaengine driver This series adds DMA Engine support for AM33xx, which uses an EDMA DMAC. The EDMA DMAC has been previously supported by only a private API implementation (much like the situation with OMAP DMA) found on the DaVinci family of SoCs. The series applies on top of 3.7-rc1 and the following patches: - GPMC fails to reserve memory fix: http://www.spinics.net/lists/linux-omap/msg79675.html - TPS65910 regulator fix: https://patchwork.kernel.org/patch/1593651/ - dmaengine DT support from Vinod's dmaengine_dt branch in git://git.infradead.org/users/vkoul/slave-dma.git since 027478851791df751176398be02a3b1c5f6aa824 The approach taken is similar to how OMAP DMA is being converted to DMA Engine support. With the functional EDMA private API already existing in mach-davinci/dma.c, we first move that to an ARM common area so it can be shared. Adding DT and runtime PM support to the private EDMA API implementation allows it to run on AM33xx. AM33xx *only* boots using DT so we leverage Jon's generic DT DMA helpers to register EDMA DMAC with the of_dma framework and then add support for calling the dma_request_slave_channel() API to both the mmc and spi drivers. With this series both BeagleBone and the AM335x EVM have working MMC and SPI support. This is tested on BeagleBone with a SPI framebuffer driver and MMC rootfs. A trivial gpio DMA event misc driver was used to test the crossbar DMA event support. It is also tested on the AM335x EVM with the onboard SPI flash and MMC rootfs. The branch at https://github.com/ohporter/linux/tree/edma-dmaengine-v3 has the complete series, dependencies, and some test drivers/defconfigs. I didn't see all the patches that you posted on edma-dmaengine-v3 but I do seem them on edma-dmaengine-am33xx-v3 branch. I added a couple of patches to enable earlyprintk and build the DTB appended kernel image uImage-dtb.am335x-evm Here's what i see [...] [0.128831] regulator-dummy: no parameters [0.130793] NET: Registered protocol family 16 [0.131694] DMA: preallocated 256 KiB pool for atomic coherent allocations [0.133030] omap-gpmc omap-gpmc: GPMC revision 6.0 [0.153136] platform 4900.edma: alias fck already exists [0.153176] platform 4900.edma: alias fck already exists [0.153199] platform 4900.edma: alias fck already exists [0.158184] OMAP GPIO hardware version 0.1 [0.172844] No ATAGs? [0.172868] hw-breakpoint: debug architecture 0x4 unsupported. [0.174282] genirq: Flags mismatch irq 28. (edma) vs. (edma) [0.174536] [ cut here ] [0.174576] WARNING: at kernel/irq/manage.c:1211 __free_irq+0x9c/0x1c0() [0.174586] Trying to free already-free IRQ 28 [0.174596] Modules linked in: [0.174645] [c001b0e0] (unwind_backtrace+0x0/0xf0) from [c0042900] (warn_slowpath_common+0x4c/0x64) [0.174668] [c0042900] (warn_slowpath_common+0x4c/0x64) from [c00429ac] (warn_slowpath_fmt+0x30/0x40) [0.174688] [c00429ac] (warn_slowpath_fmt+0x30/0x40) from [c00a8298] (__free_irq+0x9c/0x1c0) [0.174708] [c00a8298] (__free_irq+0x9c/0x1c0) from [c00a8408] (free_irq+0x4c/0xa8) [0.174739] [c00a8408] (free_irq+0x4c/0xa8) from [c07976ec] (edma_probe+0x93c/0xa24) [0.174770] [c07976ec] (edma_probe+0x93c/0xa24) from [c03226d4] (platform_drv_probe+0x18/0x1c) [0.174793] [c03226d4] (platform_drv_probe+0x18/0x1c) from [c0321494] (driver_probe_device+0x84/0x224) [0.174814] [c0321494] (driver_probe_device+0x84/0x224) from [c03216c8] (__driver_attach+0x94/0x98) [0.174834] [c03216c8] (__driver_attach+0x94/0x98) from [c031fd60] (bus_for_each_dev+0x50/0x7c) [0.174854] [c031fd60] (bus_for_each_dev+0x50/0x7c) from [c0320bc4] (bus_add_driver+0xa0/0x240) [0.174874] [c0320bc4] (bus_add_driver+0xa0/0x240) from [c0321bfc] (driver_register+0x78/0x144) [0.174896] [c0321bfc]
RE: [RFC PATCH v3 00/16] DMA Engine support for AM33XX
On Fri, Oct 19, 2012 at 16:45:58, Porter, Matt wrote: On Fri, Oct 19, 2012 at 10:26:20AM +, Bedia, Vaibhav wrote: [...] I didn't see all the patches that you posted on edma-dmaengine-v3 but I do seem them on edma-dmaengine-am33xx-v3 branch. I see I referenced the wrong branch in the cover letter. Thanks for testing and noticing this. Sorry to make you hunt for the correct branch in that repo. ;) No problem. https://github.com/ohporter/linux/tree/edma-dmaengine-am33xx-v3 is indeed the correct branch for those wanting to pull this in or grab some of the not-to-be-merged drivers I used for testing. I added a couple of patches to enable earlyprintk and build the DTB appended kernel image uImage-dtb.am335x-evm Here's what i see [...] snip [0.175354] edma: probe of 4900.edma failed with error -16 I missed an uninitialized pdata case in the bug fixes mentioned in the changelog and the folks previously failing the same way didn't hit the case I suspect you are hitting. Can you try this and let me know how it works? That doesn't help :( Looking at the original crash log, I suspect something is not correct with the irq portion, probably in the DT or the driver. genirq: Flags mismatch irq 28. (edma) vs. (edma) The warning below that is coming due to fail case in edma_probe not tracking the request_irq status properly and but IMO that's a separate issue. BTW, I am trying this on the EVM. Regards, Vaibhav -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [RFC] ARM: sched_clock: update epoch_cyc on resume
On Sat, Jul 28, 2012 at 03:53:05, Linus Walleij wrote: > On Tue, Jul 24, 2012 at 11:16 AM, Bedia, Vaibhav wrote: > > >> > A connecting theme is that of being avle to flag clock sources as > >> > sched_clock providers. If all clocksources were tagged with > >> > rating, and only clocksources were used for sched_clock(), the > >> > kernel could select the highest-rated clock under all circumstances. > >> > > >> > But that's quite intrusive, more of an idea. :-P > >> > > > > Too intrusive I guess ;) > > > > There were some discussions on this in the context of AM335x [1] [2]. > > Right now only sched_clock can be registered and I guess this restriction > > is not going to go away any time soon. > > Why do you think that? The restriction to only assign sched_clock() at > compile-time was recently removed so its now runtime assigned. > Yes it's runtime assigned but multiple calls to setup_sched_clock() will trigger a WARN_ON() from arch/arm/kernel/sched_clock.c unless I missing something basic here. > So yes, a clock source that can die and change frequency is no good > as primary system time, but the abstraction could still be used for > those that do, just add another flag NOT_CONTINUOUS or so, and > make sure the system does not select this for primary system > clock source. > > Then modelling sched_clock() on clock sources makes all more > sense: just select the best one. For primary system clock source, > do not select one which is non-continous. > I think what you are suggesting above is similar to what AM335x was trying to do but Russell pointed out issues with switching of timing sources and that's what I was trying to highlight. Regards, Vaibhav B. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [RFC] ARM: sched_clock: update epoch_cyc on resume
On Sat, Jul 28, 2012 at 03:53:05, Linus Walleij wrote: On Tue, Jul 24, 2012 at 11:16 AM, Bedia, Vaibhav vaibhav.be...@ti.com wrote: A connecting theme is that of being avle to flag clock sources as sched_clock providers. If all clocksources were tagged with rating, and only clocksources were used for sched_clock(), the kernel could select the highest-rated clock under all circumstances. But that's quite intrusive, more of an idea. :-P Too intrusive I guess ;) There were some discussions on this in the context of AM335x [1] [2]. Right now only sched_clock can be registered and I guess this restriction is not going to go away any time soon. Why do you think that? The restriction to only assign sched_clock() at compile-time was recently removed so its now runtime assigned. Yes it's runtime assigned but multiple calls to setup_sched_clock() will trigger a WARN_ON() from arch/arm/kernel/sched_clock.c unless I missing something basic here. So yes, a clock source that can die and change frequency is no good as primary system time, but the abstraction could still be used for those that do, just add another flag NOT_CONTINUOUS or so, and make sure the system does not select this for primary system clock source. Then modelling sched_clock() on clock sources makes all more sense: just select the best one. For primary system clock source, do not select one which is non-continous. I think what you are suggesting above is similar to what AM335x was trying to do but Russell pointed out issues with switching of timing sources and that's what I was trying to highlight. Regards, Vaibhav B. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [RFC] ARM: sched_clock: update epoch_cyc on resume
On Tue, Jul 24, 2012 at 05:58:32, Colin Cross wrote: > On Mon, Jul 23, 2012 at 5:14 PM, Linus Walleij > wrote: > > On Mon, Jul 23, 2012 at 9:27 PM, Colin Cross wrote: > >> On Mon, Jul 23, 2012 at 11:55 AM, Linus Walleij > > > >> Does the clock you use for sched_clock continue to run in all suspend > >> modes? All the SoC's I've used only have a 32kHz clock in the deepest > >> suspend mode, > > > > Yes, and yes it is 32kHz. > > On the platform that I am working on (AM335x) the sched_clock continues to run and yes even on this it runs @ 32KHz. > >> which is not ideal for sched_clock. > > > > Not that I know why scheduling with 32kHz is so bad compared to the > > default system scheduling granularity which is HZ if you don't have > > sched_clock() implemented. > > > > Since this seems to be such an important point, what makes you > > want MHz:es for scheduling granularity? To me the biggest impact > > is actually the granularity of the timestamps in the printk:s. > > I personally find the timestamps in the printks very late in the suspend or very early in the resume extremely helpful for debugging. The printk timestamps also help figure out which portions of the platform suspend code are taking long and can be optimized. > > (It's not that I doubt your needs, more curiosity.) > > There's a comment somewhere about higher resolution sched_clock > providing fairer scheduling. With 32 kHz sched_clock, every runtime > measured by the scheduler will be wrong by up to 31.25 us. Most > systems have a faster clock, and if it's available it seems silly not > to use it. > > It's also used for tracing, where 31.25 us resolution is a little low > for function tracing or function graph tracing. > > >> For example, on > >> Tegra2 the faster 1MHz clock used for sched_clock resets in the > >> deepest suspend state (LP0) but not the shallowest suspend state > >> (LP2), and which suspend state the chip hits depends on which hardware > >> is active. Opting out of this patch would cause Tegra's clock to > >> sometimes run in suspend, and sometimes not, which seems worse for > >> debugging than consistently not running in suspend. I'd be surprised > >> if a similar situation didn't apply to your platform. > > Is there any other slower on Tegra2 which doesn't reset and hence can be used for sched_clock in LP0 as well as LP2? There's a trade-off that the end-user might choose to make - either use a faster clock for sched_clock which stops in suspend or use a slower, in most cases 32KHz, clock which doesn't. IMO this aspect should be configurable. > > Well being able to switch between different sched_clock() providers > > may be the ideal... > > Hmm... not exactly. More below. > >>> - If it absolutely needs to be in the core code, also have a bool > >>> field indicating whether the clock is going to die during suspend > >>> and add new registration functions for setting that sched_clock > >>> type, e.g. setup_sched_clock_nonsuspendable() > >> > >> Sounds reasonable if some platforms need the extra complexity. > > I am not sure but I am guessing even on OMAPx the timer used as sched_clock keeps running. > > OK agreed. > > > > A connecting theme is that of being avle to flag clock sources as > > sched_clock providers. If all clocksources were tagged with > > rating, and only clocksources were used for sched_clock(), the > > kernel could select the highest-rated clock under all circumstances. > > > > But that's quite intrusive, more of an idea. :-P > Too intrusive I guess ;) There were some discussions on this in the context of AM335x [1] [2]. Right now only sched_clock can be registered and I guess this restriction is not going to go away any time soon. Regards, Vaibhav B. [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-January/080862.html [2] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-April/092890.html > sched_clock is supposed to be very low overhead compared to ktime_get, > and has some strict requirements if CONFIG_HAVE_UNSTABLE_SCHED_CLOCK > is not set (see kernel/sched/clock.c), but it might be possible. > > ___ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [RFC] ARM: sched_clock: update epoch_cyc on resume
On Tue, Jul 24, 2012 at 05:58:32, Colin Cross wrote: On Mon, Jul 23, 2012 at 5:14 PM, Linus Walleij linus.wall...@linaro.org wrote: On Mon, Jul 23, 2012 at 9:27 PM, Colin Cross ccr...@android.com wrote: On Mon, Jul 23, 2012 at 11:55 AM, Linus Walleij Does the clock you use for sched_clock continue to run in all suspend modes? All the SoC's I've used only have a 32kHz clock in the deepest suspend mode, Yes, and yes it is 32kHz. On the platform that I am working on (AM335x) the sched_clock continues to run and yes even on this it runs @ 32KHz. which is not ideal for sched_clock. Not that I know why scheduling with 32kHz is so bad compared to the default system scheduling granularity which is HZ if you don't have sched_clock() implemented. Since this seems to be such an important point, what makes you want MHz:es for scheduling granularity? To me the biggest impact is actually the granularity of the timestamps in the printk:s. I personally find the timestamps in the printks very late in the suspend or very early in the resume extremely helpful for debugging. The printk timestamps also help figure out which portions of the platform suspend code are taking long and can be optimized. (It's not that I doubt your needs, more curiosity.) There's a comment somewhere about higher resolution sched_clock providing fairer scheduling. With 32 kHz sched_clock, every runtime measured by the scheduler will be wrong by up to 31.25 us. Most systems have a faster clock, and if it's available it seems silly not to use it. It's also used for tracing, where 31.25 us resolution is a little low for function tracing or function graph tracing. For example, on Tegra2 the faster 1MHz clock used for sched_clock resets in the deepest suspend state (LP0) but not the shallowest suspend state (LP2), and which suspend state the chip hits depends on which hardware is active. Opting out of this patch would cause Tegra's clock to sometimes run in suspend, and sometimes not, which seems worse for debugging than consistently not running in suspend. I'd be surprised if a similar situation didn't apply to your platform. Is there any other slower on Tegra2 which doesn't reset and hence can be used for sched_clock in LP0 as well as LP2? There's a trade-off that the end-user might choose to make - either use a faster clock for sched_clock which stops in suspend or use a slower, in most cases 32KHz, clock which doesn't. IMO this aspect should be configurable. Well being able to switch between different sched_clock() providers may be the ideal... Hmm... not exactly. More below. - If it absolutely needs to be in the core code, also have a bool field indicating whether the clock is going to die during suspend and add new registration functions for setting that sched_clock type, e.g. setup_sched_clock_nonsuspendable() Sounds reasonable if some platforms need the extra complexity. I am not sure but I am guessing even on OMAPx the timer used as sched_clock keeps running. OK agreed. A connecting theme is that of being avle to flag clock sources as sched_clock providers. If all clocksources were tagged with rating, and only clocksources were used for sched_clock(), the kernel could select the highest-rated clock under all circumstances. But that's quite intrusive, more of an idea. :-P Too intrusive I guess ;) There were some discussions on this in the context of AM335x [1] [2]. Right now only sched_clock can be registered and I guess this restriction is not going to go away any time soon. Regards, Vaibhav B. [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-January/080862.html [2] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-April/092890.html sched_clock is supposed to be very low overhead compared to ktime_get, and has some strict requirements if CONFIG_HAVE_UNSTABLE_SCHED_CLOCK is not set (see kernel/sched/clock.c), but it might be possible. ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
