Re: [rtc-linux] [PATCH] rtc: OMAP: Add external 32k clock feature
On Fri, 10 Apr 2015 13:56:54 +0530 Keerthy a0393...@ti.com wrote: How do we know that all systems have this external clock and that it works OK? AM335 and AM43X have the external clock feature which we choose using RTC_OSC_REG. I verified it works OK by seeing the RTC seconds ticking even after switching the source to the external 32k Clock. AFAIU, this is related to the external (outside of SoC) oscillator, right? If so, there is a possibility to not assemble it on the board (at least on AM335) and use the internal clock source instead. Yes. The register 'OMAP_RTC_OSC_REG' is part of the RTC IP register set so i assumed that it would be with the RTCs of those particular SoCs. I'm a bit lost here. AFAICT there's a risk that some manufacturers have not wired up the external oscillator, so this patch will break those systems. Do we know for sure that this cannot be the case? Is there any way in which we can get all systems working properly? If there's no way of auto-detecting an external oscillator then perhaps a module parameter is needed. If so, it would be better if the driver were to default to internal oscillator (ie: current behaviour), and the module parameter selects the external oscillator. This way, existing systems are unaffected by the kernel upgrade. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mm/migrate: Mark unmap_and_move() noinline to avoid ICE in gcc 4.7.3
On Tue, 7 Apr 2015 10:57:52 -0700 Kevin Hilman khil...@kernel.org wrote: The diff below[2] on top of yours compiles fine here and at least covers the compilers I *know* to trigger the ICE. I see my fix in your mmots since last Thurs (4/2), but it's not in mmotm (last updated today) so today's linux-next still has the ICE for anything other than gcc-4.7.3. Just checking to see when you plan to update mmotm. It should all be there today? -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mm/migrate: Mark unmap_and_move() noinline to avoid ICE in gcc 4.7.3
On Tue, 07 Apr 2015 15:41:32 -0700 Kevin Hilman khil...@kernel.org wrote: Andrew Morton a...@linux-foundation.org writes: On Tue, 7 Apr 2015 10:57:52 -0700 Kevin Hilman khil...@kernel.org wrote: The diff below[2] on top of yours compiles fine here and at least covers the compilers I *know* to trigger the ICE. I see my fix in your mmots since last Thurs (4/2), but it's not in mmotm (last updated today) so today's linux-next still has the ICE for anything other than gcc-4.7.3. Just checking to see when you plan to update mmotm. It should all be there today? Nope. huh, I swear I did an mmotm yesterday. Let me see if I can sort out the watchdog mess and produce something releasable... -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mm/migrate: Mark unmap_and_move() noinline to avoid ICE in gcc 4.7.3
On Tue, 07 Apr 2015 16:27:44 -0700 Kevin Hilman khil...@kernel.org wrote: It should all be there today? Nope. huh, I swear I did an mmotm yesterday. Well, based on the timestamp of the mmotm dir on ozlabs, it appears you did. That's why I was confused why the gcc-473 patches from mmots aren't there. Things look a bit better now. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mm/migrate: Mark unmap_and_move() noinline to avoid ICE in gcc 4.7.3
On Wed, 01 Apr 2015 10:47:49 +0100 Marc Zyngier marc.zyng...@arm.com wrote:
-static int unmap_and_move(new_page_t get_new_page, free_page_t
put_new_page,
- unsigned long private, struct page *page, int force,
- enum migrate_mode mode)
+static noinline int unmap_and_move(new_page_t get_new_page,
+ free_page_t put_new_page,
+ unsigned long private, struct page *page,
+ int force, enum migrate_mode mode)
{
int rc = 0;
int *result = NULL;
Ouch. That's really ugly. And on 32bit ARM, we end-up spilling half of
the parameters on the stack, which is not going to help performance
either (not that this would be useful on 32bit ARM anyway...).
Any chance you could make this dependent on some compiler detection
mechanism?
With my arm compiler (gcc-4.4.4) the patch makes no difference -
unmap_and_move() isn't being inlined anyway.
How does this look?
Kevin, could you please retest? I might have fat-fingered something...
---
a/mm/migrate.c~mm-migrate-mark-unmap_and_move-noinline-to-avoid-ice-in-gcc-473-fix
+++ a/mm/migrate.c
@@ -901,10 +901,20 @@ out:
}
/*
+ * gcc-4.7.3 on arm gets an ICE when inlining unmap_and_move(). Work around
+ * it.
+ */
+#if GCC_VERSION == 40703 defined(CONFIG_ARM)
+#define ICE_noinline noinline
+#else
+#define ICE_noinline
+#endif
+
+/*
* Obtain the lock on page, remove all ptes and migrate the page
* to the newly allocated page in newpage.
*/
-static noinline int unmap_and_move(new_page_t get_new_page,
+static ICE_noinline int unmap_and_move(new_page_t get_new_page,
free_page_t put_new_page,
unsigned long private, struct page *page,
int force, enum migrate_mode mode)
_
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: musb: Fix fifo reads for dm816x with musb_dsps
On Wed, 18 Mar 2015 15:48:02 -0700 Tony Lindgren t...@atomide.com wrote:
Looks like dm81xx can only do 32-bit fifo reads like am35x. Let's set
up musb-dsps with a custom read_fifo function based on the compatible
flag.
Otherwise we can get the following errors when starting dhclient on a
asix USB Ethernet adapter:
asix 2-1:1.0 eth2: asix_rx_fixup() Bad Header Length 0x003c, offset 4
While at it, let's also remove pointless cast of the driver data.
This breaks my i386 allmodconfig build.
+/* Similar to am35x, dm81xx support only 32-bit read operation */
+static void dsps_read_fifo32(struct musb_hw_ep *hw_ep, u16 len, u8 *dst)
+{
+ void __iomem *fifo = hw_ep-fifo;
+ u32 val;
+ int i;
+
+ /* Read for 32bit-aligned destination address */
+ if (likely((0x03 (unsigned long)dst) == 0) len = 4) {
+ readsl(fifo, dst, len 2);
akpm3:/usr/src/linux-4.0-rc6 grep -r readsl arch/x86
akpm3:/usr/src/linux-4.0-rc6
+ dst += len ~0x03;
+ len = 0x03;
+ }
+ /*
+ * Now read the remaining 1 to 3 byte or complete length if
+ * unaligned address.
+ */
+ if (len 4) {
+ for (i = 0; i (len 2); i++) {
+ *(u32 *)dst = musb_readl(fifo, 0);
+ dst += 4;
+ }
+ len = 0x03;
+ }
+ if (len 0) {
+ val = musb_readl(fifo, 0);
+ memcpy(dst, val, len);
+ }
+}
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [rtc-linux] [PATCH] rtc: OMAP: Add external 32k clock feature
On Tue, 3 Mar 2015 15:12:02 +0530 Keerthy j-keer...@ti.com wrote:
Add external 32k clock feature. The internal clock will be gated during
suspend.
Hence make use of the external 32k clock so that rtc is functional accross
suspend/resume.
...
@@ -446,6 +449,7 @@ static const struct omap_rtc_device_type
omap_rtc_default_type = {
static const struct omap_rtc_device_type omap_rtc_am3352_type = {
.has_32kclk_en = true,
+ .has_osc_ext_32k = true,
.has_kicker = true,
.has_irqwakeen = true,
.has_pmic_mode = true,
@@ -543,7 +547,16 @@ static int __init omap_rtc_probe(struct platform_device
*pdev)
if (rtc-type-has_32kclk_en) {
reg = rtc_read(rtc, OMAP_RTC_OSC_REG);
rtc_writel(rtc, OMAP_RTC_OSC_REG,
- reg | OMAP_RTC_OSC_32KCLK_EN);
+reg | OMAP_RTC_OSC_32KCLK_EN);
+ }
+
+ /* Enable External clock as the source */
+
+ if (rtc-type-has_osc_ext_32k) {
+ rtc_writel(rtc, OMAP_RTC_OSC_REG,
+(OMAP_RTC_OSC_EXT_32K |
+rtc_read(rtc, OMAP_RTC_OSC_REG))
+(~OMAP_RTC_OSC_OSC32K_GZ));
}
How do we know that all systems have this external clock and that it
works OK?
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] rtc: omap: add support for pmic_power_en
On Wed, 29 Oct 2014 13:50:05 +0100 Johan Hovold jo...@kernel.org wrote: On Tue, Oct 28, 2014 at 09:36:33AM +0100, Johan Hovold wrote: On Mon, Oct 27, 2014 at 03:40:31PM -0700, Andrew Morton wrote: On Mon, 27 Oct 2014 09:09:28 +0100 Johan Hovold jo...@kernel.org wrote: But in general, how do you want to handle updates to a single patch in a series you already have in your tree? Do you prefer a proper incremental-fix patch (with commit message), just an updated single patch, or a resend of the whole series? How should I best send the updated patch? Can you just replace the current three incremental patches: rtc-omap-add-support-for-pmic_power_en.patch rtc-omap-add-support-for-pmic_power_en-v3.patch rtc-omap-add-support-for-pmic_power_en-v3-fix.patch that you have in your tree, with a single new v4 which adds a more elaborate comment? Yep, that works. I'll almost always turn the new patch into an incremental, mainly so that I (and others) can see what was changed. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] rtc: omap: add support for pmic_power_en
On Tue, 28 Oct 2014 09:36:33 +0100 Johan Hovold jo...@kernel.org wrote: But it doesn't explain *why* we want the alarm to trigger before returning. Should we really require every power-off handler to document arch behaviour (even if its inconsistent and currently undocumented); in this case that some arches return to user-space where we may oops if called from process 0 (e.g. systemd, but not if using sysvinit)? The kernel really doesn't have a problem related to excessive amounts of useful code comments :( The bottom line is: did I provide a reader with the ability to understand why the code is this way? If no then improvements beckon. This does look like one code site where an elaborate explanation is warranted. There's no way in which a reader can get to your above paragraph from the current rtc-omap.c. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] rtc: omap: add support for pmic_power_en
On Mon, 27 Oct 2014 09:09:28 +0100 Johan Hovold jo...@kernel.org wrote:
Add new property ti,system-power-controller to register the RTC as a
power-off handler.
Some RTC IP revisions can control an external PMIC via the pmic_power_en
pin, which can be configured to transition to OFF on ALARM2 events and
back to ON on subsequent ALARM (wakealarm) events.
This is based on earlier work by Colin Foe-Parker and AnilKumar Ch. [1]
[1] https://www.mail-archive.com/linux-omap@vger.kernel.org/msg82127.html
Tested-by: Felipe Balbi ba...@ti.com
Signed-off-by: Johan Hovold jo...@kernel.org
---
Changes since v2:
- add two-second delay to allow alarm to trigger before returning
hmpf. As this sentence is below the ^--- it doesn't get into the
changelog.
...
+static void omap_rtc_power_off(void)
+{
+ struct omap_rtc *rtc = omap_rtc_power_off_rtc;
+ struct rtc_time tm;
+ unsigned long now;
+ u32 val;
+
+ /* enable pmic_power_en control */
+ val = rtc_readl(rtc, OMAP_RTC_PMIC_REG);
+ rtc_writel(rtc, OMAP_RTC_PMIC_REG, val | OMAP_RTC_PMIC_POWER_EN_EN);
+
+ /* set alarm two seconds from now */
+ omap_rtc_read_time_raw(rtc, tm);
+ bcd2tm(tm);
+ rtc_tm_to_time(tm, now);
+ rtc_time_to_tm(now + 2, tm);
+
+ if (tm2bcd(tm) 0) {
+ dev_err(rtc-rtc-dev, power off failed\n);
+ return;
+ }
+
+ rtc_wait_not_busy(rtc);
+
+ rtc_write(rtc, OMAP_RTC_ALARM2_SECONDS_REG, tm.tm_sec);
+ rtc_write(rtc, OMAP_RTC_ALARM2_MINUTES_REG, tm.tm_min);
+ rtc_write(rtc, OMAP_RTC_ALARM2_HOURS_REG, tm.tm_hour);
+ rtc_write(rtc, OMAP_RTC_ALARM2_DAYS_REG, tm.tm_mday);
+ rtc_write(rtc, OMAP_RTC_ALARM2_MONTHS_REG, tm.tm_mon);
+ rtc_write(rtc, OMAP_RTC_ALARM2_YEARS_REG, tm.tm_year);
+
+ /*
+ * enable ALARM2 interrupt
+ *
+ * NOTE: this fails on AM3352 if rtc_write (writeb) is used
+ */
+ val = rtc_read(rtc, OMAP_RTC_INTERRUPTS_REG);
+ rtc_writel(rtc, OMAP_RTC_INTERRUPTS_REG,
+ val | OMAP_RTC_INTERRUPTS_IT_ALARM2);
+
+ mdelay(2000);
And it is uncommented.
How on earth is a reader to know why this is here?
I can do this
--- a/drivers/rtc/rtc-omap.c~rtc-omap-add-support-for-pmic_power_en-v3-fix
+++ a/drivers/rtc/rtc-omap.c
@@ -417,6 +417,7 @@ static void omap_rtc_power_off(void)
rtc_writel(rtc, OMAP_RTC_INTERRUPTS_REG,
val | OMAP_RTC_INTERRUPTS_IT_ALARM2);
+ /* Allow alarm to trigger before returning */
mdelay(2000);
}
But it doesn't explain *why* we want the alarm to trigger before
returning.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/20] rtc: omap: fixes and power-off feature
On Fri, 24 Oct 2014 21:55:32 +0200 Johan Hovold jo...@kernel.org wrote: On Fri, Oct 24, 2014 at 02:44:42PM -0500, Felipe Balbi wrote: On Fri, Oct 24, 2014 at 09:36:55PM +0200, Johan Hovold wrote: On Fri, Oct 24, 2014 at 02:29:48PM -0500, Felipe Balbi wrote: Hi, On Fri, Oct 24, 2014 at 02:25:40PM -0500, Felipe Balbi wrote: with this I always get to Power off failed -- system halted. If I switch to v3.18-rc1 vanilla, then it works. So it's definitely caused by your rtc-only patches. That's expected (see below). It works with v3.18-rc1 vanilla because machine_halt is called instead of machine_power_off as there is no registered power-off handler. yeah, that much I figured :-) ok, so it seems like it takes more than 1 second for things to propagate. If I increase that mdelay() to 3000, then everything works fine on my end. I think we should get RMK's input on this 3000ms delay to machine_power_off(). Should it be generic, or should we add it to our rtc pm_power_off implementation ? As I wrote above, we still need a 2-second mdelay in rtc-omap, which I intend to add to the pmic_power_en patch. oh, alright then. If you can Cc me, I'll make sure to test that too ;-) I will. :) Just wanted to see whether Andrew preferred I resend the whole series or just that one patch first. The diff is minimal: diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c index e74750f00b18..e4f97ad9eb21 100644 --- a/drivers/rtc/rtc-omap.c +++ b/drivers/rtc/rtc-omap.c @@ -423,6 +423,8 @@ static void omap_rtc_power_off(void) val = rtc_read(rtc, OMAP_RTC_INTERRUPTS_REG); rtc_writel(rtc, OMAP_RTC_INTERRUPTS_REG, val | OMAP_RTC_INTERRUPTS_IT_ALARM2); + + mdelay(2000); } Yes, having read this threadlet: we need a very good comment in there explaining what's going on, please. Do we even need this delay on anything other than arm? Or even on all arm? -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rtc: rtc-twl: ensure IRQ is wakeup enabled
On Fri, 31 May 2013 15:37:07 -0700 Kevin Hilman khil...@linaro.org wrote:
Currently, the RTC IRQ is never wakeup-enabled so is not capable of
bringing the system out of suspend.
On OMAP platforms, we have gotten by without this because the TWL RTC
is on an I2C-connected chip which is capable of waking up the OMAP via
the IO ring when the OMAP is in low-power states.
However, if the OMAP suspends without hitting the low-power states
(and the IO ring is not enabled), RTC wakeups will not work because
the IRQ is not wakeup enabled.
To fix, ensure the RTC IRQ is wakeup enabled whenever the RTC alarm is
set.
--- a/drivers/rtc/rtc-twl.c
+++ b/drivers/rtc/rtc-twl.c
@@ -213,12 +213,24 @@ static int mask_rtc_irq_bit(unsigned char bit)
static int twl_rtc_alarm_irq_enable(struct device *dev, unsigned enabled)
{
+ struct platform_device *pdev = to_platform_device(dev);
+ int irq = platform_get_irq(pdev, 0);
+ static bool twl_rtc_wake_enabled;
int ret;
- if (enabled)
+ if (enabled) {
ret = set_rtc_irq_bit(BIT_RTC_INTERRUPTS_REG_IT_ALARM_M);
- else
+ if (device_can_wakeup(dev) !twl_rtc_wake_enabled) {
+ enable_irq_wake(irq);
+ twl_rtc_wake_enabled = true;
+ }
+ } else {
ret = mask_rtc_irq_bit(BIT_RTC_INTERRUPTS_REG_IT_ALARM_M);
+ if (twl_rtc_wake_enabled) {
+ disable_irq_wake(irq);
+ twl_rtc_wake_enabled = false;
+ }
+ }
Why do we need this (slightly racy) logic with twl_rtc_wake_enabled?
Other drivers don't do this.
Should we test device_may_wakeup() befre running disable_irq_wake()?
Most drivers seem to do this, but it's all a bit foggy.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [rtc-linux] [PATCH v3 1/5] rtc: OMAP: Add system pm_power_off to rtc driver
On Tue, 20 Nov 2012 15:18:43 +0530
AnilKumar Ch anilku...@ti.com wrote:
From: Colin Foe-Parker colin.foepar...@logicpd.com
Add system power off control to rtc driver which is the in-charge
of controlling the BeagleBone system power. The power_off routine
can be hooked up to pm_power_off system call.
System power off sequence:-
* Set PMIC STATUS_OFF when PMIC_POWER_EN is pulled low
* Enable PMIC_POWER_EN in rtc module
* Set rtc ALARM2 time
* Enable ALARM2 interrupt
Added while (1); after the above steps to make sure that no other
process acquire cpu. Otherwise we might see an unexpected behaviour
because we are shutting down all the power rails of SoC except RTC.
Signed-off-by: Colin Foe-Parker colin.foepar...@logicpd.com
[anilku...@ti.com: move poweroff additions to rtc driver]
Signed-off-by: AnilKumar Ch anilku...@ti.com
...
+/*
+ * rtc_power_off: Set the pmic power off sequence. The RTC generates
+ * pmic_pwr_enable control, which can be used to control an external
+ * PMIC.
+ */
+static void rtc_power_off(void)
+{
+ u32 val;
+ struct rtc_time tm;
+ spinlock_t lock;
What on earth?
+ unsigned long flags, time;
+
+ spin_lock_init(lock);
+
+ /* Set PMIC power enable */
+ val = readl(rtc_base + OMAP_RTC_PMIC_REG);
+ writel(val | OMAP_RTC_PMIC_POWER_EN_EN, rtc_base + OMAP_RTC_PMIC_REG);
+
+ /* Read rtc time */
+ omap_rtc_read_time(NULL, tm);
+
+ /* Convert Gregorian date to seconds since 01-01-1970 00:00:00 */
+ rtc_tm_to_time(tm, time);
+
+ /* Add shutdown time to the current value */
+ time += SHUTDOWN_TIME_SEC;
+
+ /* Convert seconds since 01-01-1970 00:00:00 to Gregorian date */
+ rtc_time_to_tm(time, tm);
+
+ if (tm2bcd(tm) 0)
+ return;
+
+ pr_info(System will go to power_off state in approx. %d secs\n,
+ SHUTDOWN_TIME_SEC);
+
+ /*
+ * pmic_pwr_enable is controlled by means of ALARM2 event. So here
+ * programming alarm2 expiry time and enabling alarm2 interrupt
+ */
+ rtc_write(tm.tm_sec, OMAP_RTC_ALARM2_SECONDS_REG);
+ rtc_write(tm.tm_min, OMAP_RTC_ALARM2_MINUTES_REG);
+ rtc_write(tm.tm_hour, OMAP_RTC_ALARM2_HOURS_REG);
+ rtc_write(tm.tm_mday, OMAP_RTC_ALARM2_DAYS_REG);
+ rtc_write(tm.tm_mon, OMAP_RTC_ALARM2_MONTHS_REG);
+ rtc_write(tm.tm_year, OMAP_RTC_ALARM2_YEARS_REG);
+
+ /* Enable alarm2 interrupt */
+ val = readl(rtc_base + OMAP_RTC_INTERRUPTS_REG);
+ writel(val | OMAP_RTC_INTERRUPTS_IT_ALARM2,
+ rtc_base + OMAP_RTC_INTERRUPTS_REG);
+
+ /* Do not allow to execute any other task */
+ spin_lock_irqsave(lock, flags);
+ while (1);
I suspect this doesn't do what you want it to do.
Firstly, please provide adequate code comments here so that code
readers do not also need to be mind readers.
If you want to stop this CPU dead in its tracks (why?) then
local_irq_disable();
while (1)
; /* Note correct code layout */
will do it. But it means that the NMI watchdog (if present) will come
along and whack the machine in the head a few seconds later. And this
does nothing to stop other CPUs.
But not being a mind reader, I'm really at a loss to suggest what
should be done here.
+}
...
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/4] lis3: lis3lv02d_i2c: Add device tree support
On Wed, 26 Sep 2012 13:24:00 -0700 Greg KH gre...@linuxfoundation.org wrote: On Mon, Sep 17, 2012 at 12:53:18PM +0530, AnilKumar Ch wrote: Adds device tree support to lis3lv02d_i2c driver. Along with this DT init is moved from core driver to individual drivers, with the current implementation some pdata is missing in lis3lv02d_i2c driver. Also adds platform data for lis331dlh driver to am335x-EVM. These patches were tested on AM335x-EVM. None of these patches apply to any tree that I know of, what did you do them against? Please redo them against the char-next branch of linux-next and resend them if you wish to see them applied. Presumably they're against eariler patches whcih you don't have. I'm presently sitting on drivers-misc-lis3lv02d-add-generic-dt-matching-code.patch drivers-misc-lis3lv02d-add-generic-dt-matching-code-fix.patch drivers-misc-lis3lv02d-lis3lv02d_spic-add-dt-matching-table-passthru-code.patch drivers-misc-lis3lv02d-lis3lv02d_spic-add-dt-matching-table-passthru-code-fix.patch drivers-misc-lis3lv02d-lis3lv02d_spic-add-dt-matching-table-passthru-code-fix-fix.patch drivers-misc-lis3lv02d-remove-lis3lv02d-driver-dt-init.patch drivers-misc-lis3lv02d-lis3lv02d_spic-add-lis3lv02d-device-tree-init.patch drivers-misc-lis3lv02d-lis3lv02d_spic-add-lis3lv02d-device-tree-init-fix.patch drivers-misc-lis3lv02d-lis3lv02d_i2cc-add-lis3lv02d-device-tree-init.patch drivers-misc-lis3lv02d-lis3lv02d_i2cc-add-lis3lv02d-device-tree-init-fix.patch drivers-misc-lis3lv02d-fix-some-comments-specific-to-lis331dlh-driver.patch -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/4] lis3: lis3lv02d_i2c: Add device tree support
On Wed, 26 Sep 2012 13:44:03 -0700 Greg KH gre...@linuxfoundation.org wrote: On Wed, Sep 26, 2012 at 01:34:50PM -0700, Andrew Morton wrote: On Wed, 26 Sep 2012 13:24:00 -0700 Greg KH gre...@linuxfoundation.org wrote: On Mon, Sep 17, 2012 at 12:53:18PM +0530, AnilKumar Ch wrote: Adds device tree support to lis3lv02d_i2c driver. Along with this DT init is moved from core driver to individual drivers, with the current implementation some pdata is missing in lis3lv02d_i2c driver. Also adds platform data for lis331dlh driver to am335x-EVM. These patches were tested on AM335x-EVM. None of these patches apply to any tree that I know of, what did you do them against? Please redo them against the char-next branch of linux-next and resend them if you wish to see them applied. Presumably they're against eariler patches whcih you don't have. I'm presently sitting on drivers-misc-lis3lv02d-add-generic-dt-matching-code.patch drivers-misc-lis3lv02d-add-generic-dt-matching-code-fix.patch drivers-misc-lis3lv02d-lis3lv02d_spic-add-dt-matching-table-passthru-code.patch drivers-misc-lis3lv02d-lis3lv02d_spic-add-dt-matching-table-passthru-code-fix.patch drivers-misc-lis3lv02d-lis3lv02d_spic-add-dt-matching-table-passthru-code-fix-fix.patch drivers-misc-lis3lv02d-remove-lis3lv02d-driver-dt-init.patch drivers-misc-lis3lv02d-lis3lv02d_spic-add-lis3lv02d-device-tree-init.patch drivers-misc-lis3lv02d-lis3lv02d_spic-add-lis3lv02d-device-tree-init-fix.patch drivers-misc-lis3lv02d-lis3lv02d_i2cc-add-lis3lv02d-device-tree-init.patch drivers-misc-lis3lv02d-lis3lv02d_i2cc-add-lis3lv02d-device-tree-init-fix.patch drivers-misc-lis3lv02d-fix-some-comments-specific-to-lis331dlh-driver.patch Care to send them to me so that I can get them to Linus for the 3.7 merge window? OK. I do keep the char-misc tree for these types of things :) I've actually been doing this miscdev driver since day one. I'm happy to hand over the keys but please review the patches carefully and try not to miss stuff! -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/4] ARM: dts: AM33XX: Add lis331dlh device tree data to am335x-evm
On Mon, 17 Sep 2012 12:53:22 +0530 AnilKumar Ch anilku...@ti.com wrote: Add lis331dlh device tree data to am335x-evm.dts. In AM335x EVM lis331dlh accelerometer is connected to I2C2 bus. So this patch change the status of I2C2 node to okay to use I2C2 bus. Also added all the required platform data to am335x-evm. Signed-off-by: AnilKumar Ch anilku...@ti.com --- arch/arm/boot/dts/am335x-evm.dts | 39 ++ Your arch/arm/boot/dts/am335x-evm.dts differs significantly from the one in linux-next (it should not do so), so I didn't do anything with this patch. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] RTC: TWL: ensure all interrupts are disabled during probe
On Fri, 14 Sep 2012 08:33:42 -0700 Steve Sakoman st...@sakoman.com wrote: Tested-by: Steve Sakoman st...@sakoman.com Thanks. Given the tested-by's that are rolling in, I will assume that people are hitting this problem in 3.5 and perhaps earlier kernels, so I scheduled the fix for 3.6, with a -stable backport. I might have been wrong about that - in future, please do make these issues clear in the patch changelog? -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/4] lis3: lis3lv02d_i2c: Add device tree support
On Fri, 14 Sep 2012 15:31:36 + Arnd Bergmann a...@arndb.de wrote: On Friday 14 September 2012, AnilKumar Ch wrote: Adds device tree support to lis3lv02d_i2c driver. Along with this DT init is moved from core driver to individual drivers, with the current implementation some pdata is missing in lis3lv02d_i2c driver. Also adds platform data for lis331dlh driver to am335x-EVM. These patches were tested on AM335x-EVM. No further comments on this version besides the ones I already gave on v1. My linux-arm-kernel subscription died so I don't know what your v1 comments said. Were they fatal? -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] drivers/rtc/rtc-twl.c: fix threaded IRQ to use IRQF_ONESHOT
On Fri, 6 Jul 2012 09:33:54 -0700
Kevin Hilman khil...@ti.com wrote:
Requesting a threaded interrupt without a primary handler and without
IRQF_ONESHOT is dangerous, and after commit 1c6c6952 (genirq: Reject
bogus threaded irq requests), these requests are rejected. This
causes -probe() to fail, and the RTC driver not to be availble.
To fix, add IRQF_ONESHOT to the IRQ flags.
Tested on OMAP3730/OveroSTORM and OMAP4430/Panda board using rtcwake
to wake from system suspend multiple times.
Signed-off-by: Kevin Hilman khil...@ti.com
---
Resending to broader audience and including Andrew. Since, I understand
that drivers/rtc is somewhat orphaned, Andrew, can you queue this fix for
v3.5. Thanks.
drivers/rtc/rtc-twl.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/rtc/rtc-twl.c b/drivers/rtc/rtc-twl.c
index 258abea..c5d06fe 100644
--- a/drivers/rtc/rtc-twl.c
+++ b/drivers/rtc/rtc-twl.c
@@ -510,7 +510,7 @@ static int __devinit twl_rtc_probe(struct platform_device
*pdev)
}
ret = request_threaded_irq(irq, NULL, twl_rtc_interrupt,
-IRQF_TRIGGER_RISING,
+IRQF_TRIGGER_RISING | IRQF_ONESHOT,
dev_name(rtc-dev), rtc);
if (ret 0) {
dev_err(pdev-dev, IRQ is not free.\n);
OK, this is the second such patch I've seen and it's time to wonder if
we should get grumpy at tglx. afacit 1c6c6952 broke the following
drivers:
sound/soc/codecs/wm8994.c
sound/soc/codecs/max98095.c
sound/soc/codecs/twl6040.c
drivers/usb/otg/ab8500-usb.c
drivers/usb/otg/twl4030-usb.c
drivers/gpio/gpio-sx150x.c
drivers/gpio/gpio-ab8500.c
drivers/mfd/ab8500-gpadc.c
drivers/mfd/ti-ssp.c
drivers/mfd/twl4030-madc.c
drivers/mfd/htc-i2cpld.c
drivers/mfd/wm831x-auxadc.c
drivers/mfd/twl6040-core.c
drivers/mfd/wm8350-core.c
drivers/extcon/extcon-max8997.c
drivers/mmc/host/of_mmc_spi.c
drivers/mmc/core/cd-gpio.c
drivers/net/can/mcp251x.c
drivers/nfc/pn544_hci.c
drivers/nfc/pn544.c
drivers/power/ab8500_btemp.c
drivers/power/twl4030_charger.c
drivers/power/lp8727_charger.c
drivers/power/smb347-charger.c
drivers/power/max17042_battery.c
drivers/power/wm831x_power.c
drivers/power/ab8500_fg.c
drivers/power/max8903_charger.c
drivers/power/ab8500_charger.c
drivers/regulator/wm831x-isink.c
drivers/regulator/wm831x-ldo.c
drivers/regulator/wm831x-dcdc.c
drivers/staging/ste_rmi4/synaptics_i2c_rmi4.c
drivers/staging/iio/adc/adt7310.c
drivers/staging/iio/adc/adt7410.c
drivers/staging/iio/adc/ad7816.c
drivers/staging/iio/cdc/ad7150.c
drivers/staging/iio/accel/sca3000_core.c
drivers/staging/cptm1217/clearpad_tm1217.c
drivers/input/keyboard/tc3589x-keypad.c
drivers/input/keyboard/twl4030_keypad.c
drivers/input/misc/twl4030-pwrbutton.c
drivers/input/misc/twl6040-vibra.c
drivers/input/misc/wm831x-on.c
drivers/media/radio/si470x/radio-si470x-i2c.c
drivers/base/regmap/regmap-irq.c
drivers/rtc/rtc-wm831x.c
drivers/rtc/rtc-twl.c
drivers/rtc/rtc-ab8500.c
drivers/rtc/rtc-max8998.c
drivers/rtc/rtc-isl1208.c
drivers/platform/x86/intel_mid_powerbtn.c
include/linux/mfd/wm8994/core.h
include/linux/mfd/wm8350/core.h
what am I missing here?
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] backlight: initialize struct backlight_properties properly
On Mon, 21 May 2012 09:24:35 +0200
Corentin Chary corentin.ch...@gmail.com wrote:
In all these files, the .power field was never correctly initialized.
...
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -342,6 +342,7 @@ int intel_panel_setup_backlight(struct drm_device *dev)
else
return -ENODEV;
+ memset(props, 0, sizeof(props));
This code is all still quite fragile. What happens if we add a new
field to backlight_properties? We need to go edit a zillion drivers?
There are two ways of fixing this:
a) write and use
void backlight_properties_init(struct backlight_properties *props,
int type, int max_brightness, etc);
or
b) edit all sites to do
struct backlight_properties bl_props = {
.type = BACKLIGHT_RAW,
.max_brighness = intel_panel_get_max_backlight(dev),
.power = FB_BLANK_UNBLANK,
};
they're largely equivalent - the struct will be zeroed out and
then certain fields will be initialised. a) is a bit better because
some future field may not want the all-zeroes pattern (eg, a spinlock).
But they're both better than what we have now!
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [rtc-linux] [PATCH] RTC: twl6030: correct usage of static register while reading time
On Wed, 28 Mar 2012 16:34:01 -0500 Nishanth Menon n...@ti.com wrote: From: Konstantin Shlyakhovoy x0155...@ti.com RTC stores time and date in several registers. Due to the fact that these registers can't be read instantaneously, there is a chance that reading from counting registers gives an error of one minute, one hour, one day, etc. To address this issue, the RTC has hardware support to copy the RTC counting registers to static shadowed registers. The current implementation does not use this feature, and in a stress test, we can reproduce this error at a rate of around two times per 30 readings. Fix the implementation to ensure that the right snapshot of time is captured. hey, nice changelog! Please cc me on rtc patches - I tend to be a bit slow reading the rtc-linux list. Cc: Alessandro Zummo a.zu...@towertech.it Cc: Benoit Cousson b-cous...@ti.com Cc: linux-omap linux-omap@vger.kernel.org Acked-by: Mykola Oleksiienko x0174...@ti.com Acked-by: Oleksandr Dmytryshyn oleksandr.dmytrys...@ti.com Acked-by: Graeme Gregory g...@slimlogic.co.uk Acked-by: Nishanth Menon n...@ti.com Signed-off-by: Konstantin Shlyakhovoy x0155...@ti.com I rewrote your acked-by to Signed-off-by, because you were on the delivery path of the patch. Documentation/SubmittingPatches has details. Is this OK? -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/18] led-triggers: create a trigger for CPU activity
On Fri, 30 Mar 2012 19:58:16 +0800
Bryan Wu bryan...@canonical.com wrote:
Attempting to consolidate the ARM LED code, this removes the
custom RealView LED trigger code to turn LEDs on and off in
response to CPU activity and replace it with a standard trigger.
(bryan...@canonical.com:
It moves arch/arm/kernel/leds.c syscore stubs into this trigger.
It also provides ledtrig_cpu trigger event stub in linux/leds.h.
Although it was inspired by ARM work, it can be used in other arch.)
...
+#include linux/percpu.h
+#include linux/syscore_ops.h
+#include leds.h
+
+#define MAX_NAME_LEN 8
The kernel already has at least eight different definitions of
MAX_NAME_LEN. I guess a ninth won't hurt ;)
...
+static void __init ledtrig_cpu_register(void)
+{
+ int cpuid = smp_processor_id();
+ struct led_trigger *trig;
+ char *name = __get_cpu_var(trig_name);
+
+ snprintf(name, MAX_NAME_LEN, cpu%d, cpuid);
+ led_trigger_register_simple(name, trig);
+
+ pr_info(LED trigger %s indicate activity on CPU %d\n,
+ trig-name, cpuid);
+
+ __get_cpu_var(cpu_trig) = trig;
+}
+
+static void __exit ledtrig_cpu_unregister(void)
+{
+ struct led_trigger *trig = __get_cpu_var(cpu_trig);
+ char *name = __get_cpu_var(trig_name);
+
+ led_trigger_unregister_simple(trig);
+ __get_cpu_var(cpu_trig) = NULL;
+ memset(name, 0, MAX_NAME_LEN);
+}
+
+static int __init ledtrig_cpu_init(void)
+{
+ int cpu;
+
+ for_each_possible_cpu(cpu)
+ ledtrig_cpu_register();
+
+ register_syscore_ops(ledtrig_cpu_syscore_ops);
+
+ return 0;
+}
+module_init(ledtrig_cpu_init);
This all looks horridly broken. We call ledtrig_cpu_register() once
for each CPU, but we call it on the same CPU each time, and it uses
smp_processor_id() which a) is wrong and b) will emit a runtime
preemption-is-enabled warning.
I'm assuming this wasn't tested on SMP. It needs to be, please.
Also, the code uses for_each_possible_cpu, ignoring CPU hotplug.
That's probably justifiable for this small storage size and not-hotpath
code, but the decision should be given prominence and justified in the
changelog or, better, in code comments please.
...
The patchset is basically an ARM thing. Is there some ARM tree via
which we can get it merged?
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] lis3lv02d: Add STMicroelectronics lis33ldlh digital
On Tue, 17 Jan 2012 07:32:47 +
AnilKumar, Chimata anilku...@ti.com wrote:
Hi All,
Recalling the patch, provide the comments if there are any if not please
include
this patch to v3.3 kernel.
The patch is all mangled by someone's email client. Wordwrapping, mime
crap which my MUA can't decrypt, etc.
-Original Message-
From: AnilKumar, Chimata
Sent: Friday, December 23, 2011 10:55 AM
To: a...@arndb.de; g...@kroah.com; eric.p...@tremplin-utc.net;
a...@linux-foundation.org; broo...@opensource.wolfsonmicro.com;
linux-ker...@vger.kernel.org
Cc: linux-omap@vger.kernel.org; Nori, Sekhar; AnilKumar, Chimata
A pet peeve which I haven't told anyone about. If you've cc'ed someone
on a patch then I want to cc them on the patch too. That means I have
to add their Cc: lines to the changelog. But such Cc: lines include
their real names. By omitting their real names in the above fashion,
you cause extra hassle for me. Ideally, you should add their Cc: to
the changelog as well as to the mail headers, to give thsoe people the
best chance of seeing what is happening with the patch.
..
+static ssize_t lis3lv02d_range_show(struct device *dev,
+struct device_attribute *attr, char *buf)
+{
+ lis3lv02d_sysfs_poweron(lis3_dev);
+ return sprintf(buf, %d\n, lis3_dev.g_range);
+}
Are these interfaces documented anywhere? If so, please update that
documentation. If not, why not ;)
@@ -809,15 +881,33 @@ static ssize_t lis3lv02d_rate_set(struct device *dev,
return count;
}
+static ssize_t lis3lv02d_range_set(struct device *dev,
+ struct device_attribute *attr, const char *buf,
+ size_t count)
+{
+ unsigned long range;
+
+ if (strict_strtoul(buf, 0, range))
checkpatch would have told you that strict_strtoul() is deprecated.
Please always use checkpatch.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Security] [PATCH 00/20] world-writable files in sysfs and debugfs
On Sat, 12 Mar 2011 23:23:06 +0300 Vasiliy Kulikov seg...@openwall.com wrote: Vasiliy Kulikov (20): mach-ux500: mbox-db5500: world-writable sysfs fifo file leds: lp5521: world-writable sysfs engine* files leds: lp5523: world-writable engine* sysfs files misc: ep93xx_pwm: world-writable sysfs files rtc: rtc-ds1511: world-writable sysfs nvram file scsi: aic94xx: world-writable sysfs update_bios file scsi: iscsi: world-writable sysfs priv_sess file These are still not merged :( I grabbed them and shall merge some and send others at relevant maintainers, thanks. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/4] drivers: hwspinlock: add framework
On Mon, 31 Jan 2011 12:33:41 +0200 Ohad Ben-Cohen o...@wizery.com wrote: Add a platform-independent hwspinlock framework. Hardware spinlock devices are needed, e.g., in order to access data that is shared between remote processors, that otherwise have no alternative mechanism to accomplish synchronization and mutual exclusion operations. Signed-off-by: Ohad Ben-Cohen o...@wizery.com Cc: Hari Kanigeri h-kanige...@ti.com Cc: Benoit Cousson b-cous...@ti.com Cc: Kevin Hilman khil...@ti.com Cc: Grant Likely grant.lik...@secretlab.ca Cc: Arnd Bergmann a...@arndb.de Cc: Paul Walmsley p...@pwsan.com Acked-by: Tony Lindgren t...@atomide.com --- Documentation/hwspinlock.txt | 299 ++ drivers/Kconfig |2 + drivers/Makefile |2 + drivers/hwspinlock/Kconfig | 13 + drivers/hwspinlock/Makefile |5 + drivers/hwspinlock/hwspinlock.h | 61 drivers/hwspinlock/hwspinlock_core.c | 557 ++ include/linux/hwspinlock.h | 298 ++ It's a little irritating having two hwspinlock.h's. hwspinlock_internal.h wold be a conventional approach. But it's not a big deal. ... +/* + * A radix tree is used to maintain the available hwspinlock instances. + * The tree associates hwspinlock pointers with their integer key id, + * and provides easy-to-use API which makes the hwspinlock core code simple + * and easy to read. + * + * Radix trees are quick on lookups, and reasonably efficient in terms of + * storage, especially with high density usages such as this framework + * requires (a continuous range of integer keys, beginning with zero, is + * used as the ID's of the hwspinlock instances). + * + * The radix tree API supports tagging items in the tree, which this + * framework uses to mark unused hwspinlock instances (see the + * HWSPINLOCK_UNUSED tag above). As a result, the process of querying the + * tree, looking for an unused hwspinlock instance, is now reduced to a + * single radix tree API call. + */ Nice comment! +static RADIX_TREE(hwspinlock_tree, GFP_KERNEL); + ... +/** + * __hwspin_lock_timeout() - lock an hwspinlock with timeout limit + * @hwlock: the hwspinlock to be locked + * @timeout: timeout value in jiffies hm, why in jiffies? The problem here is that lazy programmers will use hwspin_lock_timeout(lock, 10, ...) and their code will work happily with HZ=100 but will explode with HZ=1000. IOW, this interface *requires* that all callers perform a seconds-to-jiffies conversion before calling hwspin_lock_timeout(). So why not reduce their effort and their ability to make mistakes by defining the API to take seconds? + * @mode: mode which controls whether local interrupts are disabled or not + * @flags: a pointer to where the caller's interrupt state will be saved at (if + * requested) + * + * This function locks the given @hwlock. If the @hwlock + * is already taken, the function will busy loop waiting for it to + * be released, but give up when @timeout jiffies have elapsed. If @timeout + * is %MAX_SCHEDULE_TIMEOUT, the function will never give up (therefore if a + * faulty remote core never releases the @hwlock, it will deadlock). + * + * Upon a successful return from this function, preemption is disabled + * (and possibly local interrupts, too), so the caller must not sleep, + * and is advised to release the hwspinlock as soon as possible. + * This is required in order to minimize remote cores polling on the + * hardware interconnect. + * + * The user decides whether local interrupts are disabled or not, and if yes, + * whether he wants their previous state to be saved. It is up to the user + * to choose the appropriate @mode of operation, exactly the same way users + * should decide between spin_lock, spin_lock_irq and spin_lock_irqsave. + * + * Returns 0 when the @hwlock was successfully taken, and an appropriate + * error code otherwise (most notably -ETIMEDOUT if the @hwlock is still + * busy after @timeout meets jiffies). The function will never sleep. + */ ... -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/4] drivers: hwspinlock: add framework
On Tue, 1 Feb 2011 08:20:13 +0200 Ohad Ben-Cohen o...@wizery.com wrote: On Tue, Feb 1, 2011 at 1:38 AM, Andrew Morton a...@linux-foundation.org wrote: It's a little irritating having two hwspinlock.h's. hwspinlock_internal.h wold be a conventional approach. __But it's not a big deal. ... +/** + * __hwspin_lock_timeout() - lock an hwspinlock with timeout limit + * @hwlock: the hwspinlock to be locked + * @timeout: timeout value in jiffies hm, why in jiffies? The problem here is that lazy programmers will use __ __ __ __hwspin_lock_timeout(lock, 10, ...) and their code will work happily with HZ=100 but will explode with HZ=1000. IOW, this interface *requires* that all callers perform a seconds-to-jiffies conversion before calling hwspin_lock_timeout(). __So why not reduce their effort and their ability to make mistakes by defining the API to take seconds? I considered that, but then decided to use jiffies in order to be consistent with wait_event_timeout/schedule_timeout (although I don't return the remaining jiffies in case the lock is taken before the timeout elapses), and also to allow user-selected granularity. But I do kind of like the idea of not using jiffies. We can probably even move to msecs, since anyway this is an error condition, and people who needs a quick check should just use the trylock() version. I'll do a quick respin of the patches with that and the hwspinlock_internal.h comment above. OK.. The patch series looks OK to me. But there isn't a lot of point in me putting them into my tree. Maybe Tony or Russell or Greg can grab them if they like the look of it all? -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/4] drivers: hwspinlock: add framework
On Tue, 1 Feb 2011 09:36:22 +0200 Ohad Ben-Cohen o...@wizery.com wrote: On Tue, Feb 1, 2011 at 8:38 AM, Andrew Morton a...@linux-foundation.org wrote: I'll do a quick respin of the patches with that and the hwspinlock_internal.h comment above. OK.. The patch series looks OK to me. Can I add your Acked-by on the non-omap parts when I respin the series ? spose so. I don't normally do acked-by's. I think it's my way of avoiding getting blamed when it all blows up. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND #2] [PATCH v2] LEDS: Add output invertion option to backlight trigger
On Thu, 9 Dec 2010 14:41:50 +0100
Janusz Krzysztofik jkrzy...@tis.icnet.pl wrote:
This patch extends the LED backlight tirgger driver with an option that
allows
for inverting the trigger output polarity.
With the invertion option provided, I (ab)use the backlight trigger for
driving a LED that indicates LCD display blank condtition on my Amstrad Delta
videophone. Since the machine has no dedicated power LED, it was not possible
to distinguish if the display was blanked, or the machine was turned off,
without touching it.
The invert sysfs control is patterned after a similiar function of the GPIO
trigger driver.
Created and tested against linux-2.6.36-rc5 on Amstrad Delta.
Retested on linux-2.6.37-rc4.
Signed-off-by: Janusz Krzysztofik jkrzy...@tis.icnet.pl
Cc: Richard Purdie rpur...@rpsys.net
---
Resent because I still can't see any response received, while yet another
merge window is going to pass away soon.
Applies cleanly on top of 2.6.37-rc4, so no need for yet another refresh.
Only
tried to clean up the commit message slightly - maybe my English is not good
enough to bother with, if not the code?
v1 - v2 changes:
- improve some conditional expressions to be more readable; thanks to Ralph
Corderoy (from e3-hacking) and Lars-Peter Clausen for their suggestions,
- refresh against linux-2.6.36-rc5.
drivers/leds/ledtrig-backlight.c | 60
---
1 file changed, 56 insertions(+), 4 deletions(-)
diff -upr linux-2.6.36-rc5.orig/drivers/leds/ledtrig-backlight.c
linux-2.6.36-rc5/drivers/leds/ledtrig-backlight.c
--- linux-2.6.36-rc5.orig/drivers/leds/ledtrig-backlight.c2010-09-24
15:35:13.0 +0200
+++ linux-2.6.36-rc5/drivers/leds/ledtrig-backlight.c 2010-10-03
15:59:49.0 +0200
@@ -26,6 +26,7 @@ struct bl_trig_notifier {
int brightness;
int old_status;
struct notifier_block notifier;
+ unsigned invert;
};
static int fb_notifier_callback(struct notifier_block *p,
@@ -36,23 +37,63 @@ static int fb_notifier_callback(struct n
struct led_classdev *led = n-led;
struct fb_event *fb_event = data;
int *blank = fb_event-data;
+ int new_status = *blank ? BLANK : UNBLANK;
switch (event) {
case FB_EVENT_BLANK :
- if (*blank n-old_status == UNBLANK) {
+ if (new_status == n-old_status)
+ break;
+
+ if ((n-old_status == UNBLANK) ^ n-invert) {
n-brightness = led-brightness;
led_set_brightness(led, LED_OFF);
- n-old_status = BLANK;
- } else if (!*blank n-old_status == BLANK) {
+ } else {
led_set_brightness(led, n-brightness);
- n-old_status = UNBLANK;
}
+
+ n-old_status = new_status;
+
break;
}
return 0;
}
+static ssize_t bl_trig_invert_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct led_classdev *led = dev_get_drvdata(dev);
+ struct bl_trig_notifier *n = led-trigger_data;
+
+ return sprintf(buf, %s\n, n-invert ? yes : no);
+}
I think this should show 0 or 1, to match the thing which the user
wrote here.
+static ssize_t bl_trig_invert_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t num)
+{
+ struct led_classdev *led = dev_get_drvdata(dev);
+ struct bl_trig_notifier *n = led-trigger_data;
+ unsigned invert;
+ int ret;
+
+ ret = sscanf(buf, %u, invert);
Here we should use strict_strtoul() so the kernel correctly rejects
input of the form 42foo.
+ if (ret 1) {
+ dev_err(dev, invalid value\n);
+ return -EINVAL;
+ }
And here it would be better to disallow any input other than 0 or 1.
Because 2 makes no sense and who knows, some time in the future we
might *want* to permit 2.
So...
---
a/drivers/leds/ledtrig-backlight.c~leds-add-output-inversion-option-to-backlight-trigger-fix
+++ a/drivers/leds/ledtrig-backlight.c
@@ -65,7 +65,7 @@ static ssize_t bl_trig_invert_show(struc
struct led_classdev *led = dev_get_drvdata(dev);
struct bl_trig_notifier *n = led-trigger_data;
- return sprintf(buf, %s\n, n-invert ? yes : no);
+ return sprintf(buf, %u\n, n-invert);
}
static ssize_t bl_trig_invert_store(struct device *dev,
@@ -73,16 +73,17 @@ static ssize_t bl_trig_invert_store(stru
{
struct led_classdev *led = dev_get_drvdata(dev);
struct bl_trig_notifier *n = led-trigger_data;
- unsigned invert;
+ unsigned long invert;
int ret;
- ret = sscanf(buf, %u, invert);
- if (ret 1) {
- dev_err(dev, invalid value\n);
+ ret = strict_strtoul(buf, 10, invert);
+ if (ret 0)
+ return ret;
+
Re: [RESEND #2] [PATCH v2] LEDS: Add output invertion option to backlight trigger
On Thu, 6 Jan 2011 13:08:56 -0800 Randy Dunlap randy.dun...@oracle.com wrote: On Thu, 6 Jan 2011 13:04:40 -0800 Andrew Morton wrote: On Thu, 9 Dec 2010 14:41:50 +0100 Janusz Krzysztofik jkrzy...@tis.icnet.pl wrote: +static DEVICE_ATTR(invert, 0644, bl_trig_invert_show, bl_trig_invert_store); This new sysfs file should be documented. Where would be an appropriate place for that? Documentation/leds-class.txt doesn't mention a sysfs API at all. -- in Documentation/ABI/, where all sysfs interface info lives. Spose so. Documentation/ABI/stable/sysfs-class-backlight does have some stuff in it. Personally I tend to regard Documentation/ABI/ as fairly useless incomprehensible stuff, maintained to keep Greg happy ;) It'd be better to have a nice little well-maintained document for a subsystem such as this which actually explains its operation in a useful-to-humans way. Rather than just mechanically filling out forms. But a Documentation/ABI update is a heck of a lot better than nothing. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/4] Introduce hardware spinlock framework
On Tue, 4 Jan 2011 14:23:20 +0200 Ohad Ben-Cohen o...@wizery.com wrote: Hi Andrew, On Sat, Dec 18, 2010 at 2:53 AM, Tony Lindgren t...@atomide.com wrote: * Ohad Ben-Cohen o...@wizery.com [101216 13:34]: Tony, Andrew, can you please have a look ? Any comment or suggestion is appreciated. Looks sane to me from omap point of view and it seems the locks are now all timeout based: Acked-by: Tony Lindgren t...@atomide.com Can you please have a look at this patch set (see link no. [1] below) ? I looked - it looks reasonable. This is exactly the wrong time to be looking at large new patchsets - please refresh, retest and resend after -rc1 is released? -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] add BUG_ON_MAPPABLE_NULL macro
On Sun, 5 Dec 2010 12:06:03 +0200
Ohad Ben-Cohen o...@wizery.com wrote:
Introduce BUG_ON_MAPPABLE_NULL in order to eliminate redundant BUG_ON
code, checking for NULL addresses, on architectures where the zero
address can never be mapped.
Originally proposed by Russell King li...@arm.linux.org.uk
Signed-off-by: Ohad Ben-Cohen o...@wizery.com
---
Compile tested on ARM and x86-64.
Relevant threads:
1. Original proposal by Russell - http://lkml.org/lkml/2010/1/21/238
2. Recent discussion - https://lkml.org/lkml/2010/11/26/78
Notes:
* Implementation still feels hacky, especially since we don't care about the
len param of arch_mmap_check.
* Just like BUG_ON, this new macro is compiled out on !CONFIG_BUG. We might
want to add a CONFIG_BUG commentary, so users will at least be aware of the
security implications of compiling this out.
* To get an (extremely!) rough upper bound of the profit of this macro, I did:
1. find . -name '*.[ch]' | xargs sed -i 's/BUG_ON(!/BUG_ON_MAPPABLE_NULL(!/'
2. removed some obviously bogus sed hits
With omap2plus_defconfig, uImage shrank by ~4Kb (obviously this doesn't
include the potential gain in modules)
Every time someone sends me a patch with text after the ---, I decide
it was good changelog material and I promote it to above the ---.
How's about you save us the effort :)
The changelog is a bit vague really, but the code comment explains it
all, and that's a good place to explain things.
+/**
+ * BUG_ON_MAPPABLE_NULL() - BUG_ON(condition) only if address 0 is mappable
+ * @condition: condition to check, should contain a NULL check
+ *
+ * In general, NULL dereference Oopses are not desirable, since they take
down
+ * the system with them and make the user extremely unhappy. So as a general
+ * rule drivers should avoid dereferencing NULL pointers by doing a simple
s/drivers/code/
+ * check (when appropriate), and just return an error rather than crash.
+ * This way the system, despite having reduced functionality, will just keep
+ * running rather than immediately reboot.
+ *
+ * _Critical_ kernel code, OTOH, that should not (/cannot) keep running when
+ * given an unexpected NULL pointer, should just crash. On some
architectures,
+ * a NULL dereference will always reliably produce an Oops. On others, where
+ * the zero address can be mmapped, an Oops is not guaranteed. Relying on
+ * NULL dereference Oopses to happen on these architectures might lead to
+ * data corruptions (system will keep running despite a critical bug and
+ * the results will be horribly undefined). In addition, these situations
+ * can also have security implications - we have seen several privilege
+ * escalation exploits with which an attacker gained full control over the
+ * system due to NULL dereference bugs.
yup.
+ * This macro will BUG_ON if @condition is true on architectures where the
zero
+ * address can be mapped. On other architectures, where the zero address
+ * can never be mapped, this macro is compiled out. It only makes sense to
+ * use this macro if @condition contains a NULL check, in order to optimize
that
+ * check out on architectures where the zero address can never be mapped.
+ * On such architectures, those checks are not necessary, since the code
+ * itself will reliably reproduce an Oops as soon as the NULL address will
+ * be dereferenced.
+ *
+ * As with BUG_ON, use this macro only if @condition cannot be tolerated.
+ * If proceeding with degraded functionality is an option, it's much
+ * better to just simply check for @condition and return some error code
rather
+ * than crash the system.
+ */
+#define BUG_ON_MAPPABLE_NULL(cond) do { \
+ if (arch_mmap_check(0, 1, MAP_FIXED) == 0) \
+ BUG_ON(cond); \
+} while (0)
- arch_mmap_check() didn't have any documentation. Please fix?
- arch_mmap_check() is a pretty poor identifier (identifiers which
include the word check are usually poor ones). Maybe
arch_address_accessible()?
- I worry about arch_mmap_check(). Is it expected that it will
perform a runtime check, like probe_kernel_address()? Spell out the
expectations, please.
- BUG_ON_MAPPABLE_NULL() will evaluate arch_mmap_check() even if
CONFIG_BUG=n. Seems bad, depending on what those unspelled-out
expectations are!
- BUG_ON_MAPPABLE_NULL() is a mouthful. I can't immedaitely think of
anythinjg nicer :(
- Is BUG_ON_MAPPABLE_NULL() really the interface you want? Or do we
just want an interface which checks a pointer for nearly-nullness?
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 1/7] pwm: Add pwm core driver
On Tue, 5 Oct 2010 17:29:56 +0530
Arun Murthy arun.mur...@stericsson.com wrote:
The existing pwm based led and backlight driver makes use of the
pwm(include/linux/pwm.h). So all the board specific pwm drivers will
be exposing the same set of function name as in include/linux/pwm.h.
Consder a platform with multi Soc or having more than one pwm module, in
such a case, there exists more than one pwm driver for a platform. Each
of these pwm drivers export the same set of function and hence leads to
re-declaration build error.
In order to overcome this issue all the pwm drivers must register to
some core pwm driver with function pointers for pwm operations (i.e
pwm_config, pwm_enable, pwm_disable).
The clients of pwm device will have to call pwm_request, wherein
they will get the pointer to struct pwm_ops. This structure include
function pointers for pwm_config, pwm_enable and pwm_disable.
Have we worked out who will be merging this work, if it gets merged?
...
+struct pwm_dev_info {
+ struct pwm_device *pwm_dev;
+ struct list_head list;
+};
+static struct pwm_dev_info *di;
We could just do
static struct pwm_dev_info {
...
} *di;
+DECLARE_RWSEM(pwm_list_lock);
This can/should be static.
+void __deprecated pwm_free(struct pwm_device *pwm)
+{
+}
Why are we adding a new function and already deprecating it?
Probably this was already addressed in earlier review, but I'm asking
again, because there's no comment explaining the reasons. Lesson
learned, please add a comment.
Oh, I see that pwm_free() already exists. This patch adds a new copy
and doesn't remove the old function. Does this all actually work?
It still needs a comment explaining why it's deprecated.
+int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns)
+{
+ if (!pwm-pops)
+ -EFAULT;
+ return pwm-pops-pwm_config(pwm, duty_ns, period_ns);
+}
+EXPORT_SYMBOL(pwm_config);
+
+int pwm_enable(struct pwm_device *pwm)
+{
+ if (!pwm-pops)
+ -EFAULT;
+ return pwm-pops-pwm_enable(pwm);
+}
+EXPORT_SYMBOL(pwm_enable);
+
+void pwm_disable(struct pwm_device *pwm)
+{
+ if (!pwm-pops)
+ -EFAULT;
+ pwm-pops-pwm_disable(pwm);
+}
+EXPORT_SYMBOL(pwm_disable);
+
+int pwm_device_register(struct pwm_device *pwm_dev)
+{
+ struct pwm_dev_info *pwm;
+
+ down_write(pwm_list_lock);
+ pwm = kzalloc(sizeof(struct pwm_dev_info), GFP_KERNEL);
+ if (!pwm) {
+ up_write(pwm_list_lock);
+ return -ENOMEM;
+ }
The allocation attempt can be moved outside the lock, making the code
faster, cleaner and shorter.
+ pwm-pwm_dev = pwm_dev;
+ list_add_tail(pwm-list, di-list);
+ up_write(pwm_list_lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(pwm_device_register);
+
+int pwm_device_unregister(struct pwm_device *pwm_dev)
+{
+ struct pwm_dev_info *tmp;
+ struct list_head *pos, *tmp_lst;
+
+ down_write(pwm_list_lock);
+ list_for_each_safe(pos, tmp_lst, di-list) {
+ tmp = list_entry(pos, struct pwm_dev_info, list);
+ if (tmp-pwm_dev == pwm_dev) {
+ list_del(pos);
+ kfree(tmp);
+ up_write(pwm_list_lock);
+ return 0;
+ }
+ }
+ up_write(pwm_list_lock);
+ return -ENOENT;
+}
+EXPORT_SYMBOL(pwm_device_unregister);
+
+struct pwm_device *pwm_request(int pwm_id, const char *name)
+{
+ struct pwm_dev_info *pwm;
+ struct list_head *pos;
+
+ down_read(pwm_list_lock);
+ list_for_each(pos, di-list) {
+ pwm = list_entry(pos, struct pwm_dev_info, list);
+ if ((!strcmp(pwm-pwm_dev-pops-name, name))
+ (pwm-pwm_dev-pwm_id == pwm_id)) {
+ up_read(pwm_list_lock);
+ return pwm-pwm_dev;
+ }
+ }
+ up_read(pwm_list_lock);
+ return ERR_PTR(-ENOENT);
+}
+EXPORT_SYMBOL(pwm_request);
We have a new kernel-wide exported-to-modules formal API. We prefer
that such things be fully documented, please. kerneldoc is a suitable
way but please avoid falling into the kerneldoc trap of filling out
fields with obvious boilerplate and not actually telling people
anything interesting or useful.
+static int __init pwm_init(void)
+{
+ struct pwm_dev_info *pwm;
+
+ pwm = kzalloc(sizeof(struct pwm_dev_info), GFP_KERNEL);
+ if (!pwm)
+ return -ENOMEM;
+ INIT_LIST_HEAD(pwm-list);
+ di = pwm;
+ return 0;
+}
OK, this looks wrong.
AFACIT you've created a dummy pwm_dev_info as a singleton, kernel-wide
anchor for a list of all pwm_dev_info's. So this anchor pwm_dev_info
never actually gets used for anything.
The way to do this is to remove `di' altogether and instead use a
singleton, kernel-wide list_head as the anchor for all the
Re: [PATCH-V2 3/3] RTC:s35390a: Add update_irq (per Min interrupt) support
On Sat, 21 Aug 2010 18:00:29 +0530
hvaib...@ti.com wrote:
+static int s35390a_update_irq_enable(struct i2c_client *client,
+ unsigned enabled)
+{
+ struct s35390a *s35390a = i2c_get_clientdata(client);
+ char buf[1];
+
+ if (s35390a_get_reg(s35390a, S35390A_CMD_STATUS2, buf, sizeof(buf)) 0)
+ return -EIO;
+
+ /* This chip returns the bits of each byte in reverse order */
+ buf[0] = bitrev8(buf[0]);
+
+ buf[0] = ~S35390A_INT1_MODE_MASK;
+ if (enabled)
+ buf[0] |= S35390A_INT1_MODE_PMIN_STDY;
+ else
+ buf[0] |= S35390A_INT1_MODE_NOINTR;
+
+ /* This chip expects the bits of each byte in reverse order */
+ buf[0] = bitrev8(buf[0]);
grumble. We bit-reverse it, fiddle a couple of bits and then
bit-reverse it again. Lazy.
Why not leave it bit-reversed and just bit-reverse the constants?
Extra marks are awarded for coming up with a compile-time bitrev8() ;)
+ return s35390a_set_reg(s35390a, S35390A_CMD_STATUS2, buf, sizeof(buf));
+}
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/2] OMAP4 HSMMC: Adding card detect support for MMC1 Controller
On Thu, 17 Jun 2010 20:57:19 +0530 (IST) kishore kadiyala kishore.kadiy...@ti.com wrote: Adding card detect callback function which gives the status of the card .For MMC1 Controller, Card detect interrupt source is twl6030 and card present/absent status is provided by MMCCTRL register of twl6030. Signed-off-by: Kishore Kadiyala kishore.kadiy...@ti.com --- arch/arm/mach-omap2/board-4430sdp.c | 11 +-- arch/arm/mach-omap2/hsmmc.c |1 + arch/arm/plat-omap/include/plat/mmc.h |1 + drivers/mfd/twl6030-irq.c | 23 drivers/mmc/host/omap_hsmmc.c | 30 ++--- include/linux/i2c/twl.h | 46 + 6 files changed, 104 insertions(+), 8 deletions(-) I assume this depends on [PATCH v5 1/2] OMAP HSMMC: Adding a Flag to determine the type of Card detect? It's all a bit too remote from my (ever-expanding) work area, so I'll assume that Tony or someone will look after these. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/2] OMAP HSMMC: Adding a Flag to determine the type of Card detect
On Thu, 17 Jun 2010 20:56:58 +0530 (IST)
kishore kadiyala kishore.kadiy...@ti.com wrote:
--- a/arch/arm/plat-omap/include/plat/mmc.h
+++ b/arch/arm/plat-omap/include/plat/mmc.h
@@ -43,6 +43,9 @@
#define OMAP_MMC_MAX_SLOTS 2
+#define NON_GPIO 0
+#define GPIO 1
I'm counting about seven different definitions of GPIO in the kernel
already.
drivers/hwmon/it87.c:
#define GPIO0x07
drivers/media/dvb/dvb-usb/ec168.h:
GPIO = 0x04,
drivers/net/hamachi.c:
GPIO=0x6E
drivers/staging/rtl8187se/r8180_hw.h:
#define GPIO 0x91
etcetera. It's a crazy identifier to use in a header file, and
the chances of a miscompile-causing collision are increasing.
enum cd_type {
CD_TYPE_NON_GPIO = 0,
CD_TYPE_GPIO = 1,
};
perhaps?
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/2] OMAP4 HSMMC: Adding card detect support for MMC1 Controller
On Thu, 17 Jun 2010 20:57:19 +0530 (IST)
kishore kadiyala kishore.kadiy...@ti.com wrote:
Adding card detect callback function which gives the status of
the card .For MMC1 Controller, Card detect interrupt source is
twl6030 and card present/absent status is provided by MMCCTRL
register of twl6030.
...
+ int ret = -ENOSYS;
+ int ret = -ENOSYS;
ENOSYS seems an inappropriate errno to use in a driver.
ENOSYS -- The system doesn't support that function. For example, if
you call setpgid() on a system without job control, you'll get an
ENOSYS error.
I think it means you the programmer tried to do something in a syscall
which didn't make sense in this context.
I'm not sure what _is_ appropraite here. There's always EIO I guess.
ENODEV?
This happens a lot. The userspace errnos just don't map well onto
kernel-internal operations. it was a mistake - we should have defined a
kernel-internal namespace and perhaps type for such things. Oh well.
+/* Configuring Card Detect for MMC1 */
+static inline int omap4_hsmmc1_card_detect_config(void)
+{
+ int res = -1;
+ u8 reg_val = 0;
+
+ /* Unmasking the Card detect Interrupt line for MMC1 from Phoenix */
+ if (twl_class_is_6030()) {
+ twl6030_interrupt_unmask(TWL6030_MMCDETECT_INT_MASK,
+ REG_INT_MSK_LINE_B);
+ twl6030_interrupt_unmask(TWL6030_MMCDETECT_INT_MASK,
+ REG_INT_MSK_STS_B);
+ }
+
+
+ /*
+ * Intially Configuring MMC_CTRL for receving interrupts
+ * Card status on TWL6030 for MMC1
+ */
+ res = twl_i2c_read_u8(TWL6030_MODULE_ID0, reg_val, TWL6030_MMCCTRL);
+ if (res 0)
+ return res;
+ reg_val = ~VMMC_AUTO_OFF;
+ reg_val |= SW_FC;
+ twl_i2c_write_u8(TWL6030_MODULE_ID0, reg_val, TWL6030_MMCCTRL);
+
+ /* Configuring CFG_INPUT_PUPD3 */
+ res = twl_i2c_read_u8(TWL6030_MODULE_ID0, reg_val,
+ TWL6030_CFG_INPUT_PUPD3);
+ if (res 0)
+ return res;
+ reg_val = ~(MMC_PU | MMC_PD);
+ twl_i2c_write_u8(TWL6030_MODULE_ID0, reg_val, TWL6030_CFG_INPUT_PUPD3);
+ return res;
+}
This is way to large to be inlined. Why not put it in a .c file?
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [MMC] omap: Remove BUG_ON for disabled interrupts
On Sat, 29 May 2010 19:27:23 -0700 Cory Maccarrone darkstar6...@gmail.com wrote: This change removes a BUG_ON for when interrupts are disabled during an MMC request. During boot, interrupts can be disabled when a request is made, causing this bug to be triggered. In reality, there's no reason this should halt the kernel, as the driver has proved reliable in spite of disabled interrupts, and additionally, there's nothing in this code that would require interrupts to be enabled. Signed-off-by: Cory Maccarrone darkstar6...@gmail.com --- drivers/mmc/host/omap.c |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/drivers/mmc/host/omap.c b/drivers/mmc/host/omap.c index 2b28168..d98ddcf 100644 --- a/drivers/mmc/host/omap.c +++ b/drivers/mmc/host/omap.c @@ -1157,7 +1157,6 @@ static void mmc_omap_start_request(struct mmc_omap_host *host, mmc_omap_start_command(host, req-cmd); if (host-dma_in_use) omap_start_dma(host-dma_ch); - BUG_ON(irqs_disabled()); } static void mmc_omap_request(struct mmc_host *mmc, struct mmc_request *req) So we need to decide whether this should be backported into 2.6.34.x and perhaps earlier. For that decision we'll need to know the things you didn't tell us: Which drivers are affected? Under which setups is it triggering? Why aren't lots of people reporting hey my kernel went BUG? -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] misc : ROHM BH1780GLI Ambient light sensor Driver
I re-added your reviewers to the cc... On Mon, 24 May 2010 16:34:25 +0530 (IST) Hemanth V heman...@ti.com wrote: This patch adds support for ROHM BH1780GLI Ambient light sensor. BH1780 supports I2C interface. Driver supports read/update of power state and read of lux value (through SYSFS). Writing value 3 to power_state enables the sensor and current lux value could be read. There are at least two other ambient light sensor drivers: drivers/misc/isl29003.c and drivers/misc/tsl2550.c. Is there any standardisation of the ABIs whcih these drivers offer? If so, does this new driver comply with that? It would be most useful if the changelog were to fully describe the proposed kernel-userspace interface. That's the most important part of the driver, because it's the only part we can never change. There is a desultory effort to maintain sysfs API descriptions under Documentation/ABI/. I'd have thought that it would be appropriate to document this driver's ABI in there. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] misc : ROHM BH1780GLI Ambient light sensor Driver
On Tue, 1 Jun 2010 22:27:15 +0200 Daniel Mack dan...@caiaq.de wrote: On Tue, Jun 01, 2010 at 01:12:44PM -0700, Andrew Morton wrote: On Mon, 24 May 2010 16:34:25 +0530 (IST) Hemanth V heman...@ti.com wrote: This patch adds support for ROHM BH1780GLI Ambient light sensor. BH1780 supports I2C interface. Driver supports read/update of power state and read of lux value (through SYSFS). Writing value 3 to power_state enables the sensor and current lux value could be read. There are at least two other ambient light sensor drivers: drivers/misc/isl29003.c and drivers/misc/tsl2550.c. Is there any standardisation of the ABIs whcih these drivers offer? If so, does this new driver comply with that? Jonathan proposed the ALS framework for these type of devices, but it was rejected (don't know about the reasons, I didn't follow the discussions). The new idea is to put such drivers in the industrial IO subsystem, but I don't know how mature that approach is currently. For the time being, these drivers cook up whatever sysfs interface they like, and their userspace ABIs are not standardized, unfortunately. Well can we fix that? Look at the existing drivers, pick one and make this new driver provide the same interface? It's not a grand plan - more of an incremental baby step, but it's better than this random proliferation. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] misc : ROHM BH1780GLI Ambient light sensor Driver
On Tue, 01 Jun 2010 21:39:10 +0100 Jonathan Cameron ker...@jic23.retrosnub.co.uk wrote: It would be most useful if the changelog were to fully describe the proposed kernel-userspace interface. That's the most important part of the driver, because it's the only part we can never change. There is a desultory effort to maintain sysfs API descriptions under Documentation/ABI/. I'd have thought that it would be appropriate to document this driver's ABI in there. Agreed, but we get back to the debate of what we should standardise on. I'd suggest standardising on one of the existing drivers. That way we have two compliant drivers and only need to change (n-2) others. If we pick some new standard then we need to change (n) drivers. And we can't change the drivers, really. They'd all end up needing to provide two interfaces: one for the shiny-new-standard and one legacy. The main point of ALS before it died was exactly putting this standardization in place (admittedly the interface was slightly different from what we are proposing in IIO, but that was before Greg pointed out that sharing with hwmon would be a good idea!) I have to admit I'm a little loath to spend too much time on this given the amount of time wasted previously (ALS). Well, it's not a waste. This is very important! We appear to be making a big mess which we can never fix up. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] gpiolib: introduce set_debounce method
On Mon, 17 May 2010 13:02:30 +0300
felipe.ba...@nokia.com wrote:
From: Felipe Balbi felipe.ba...@nokia.com
Few architectures, like OMAP, allow you to set
a debouncing time for the gpio before generating
the IRQ. Teach gpiolib about that.
...
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1447,6 +1447,49 @@ fail:
}
EXPORT_SYMBOL_GPL(gpio_direction_output);
+/**
+ * gpio_set_debounce - sets @debounce time for a @gpio
+ * @gpio: the gpio to set debounce time
+ * @debounce: debounce time is microseconds
+ */
+int gpio_set_debounce(unsigned gpio, unsigned debounce)
+{
+ unsigned long flags;
+ struct gpio_chip*chip;
+ struct gpio_desc*desc = gpio_desc[gpio];
+ int status = -EINVAL;
+
+ spin_lock_irqsave(gpio_lock, flags);
+
+ if (!gpio_is_valid(gpio))
+ goto fail;
+ chip = desc-chip;
+ if (!chip || !chip-set || !chip-set_debounce)
+ goto fail;
+ gpio -= chip-base;
+ if (gpio = chip-ngpio)
+ goto fail;
+ status = gpio_ensure_requested(desc, gpio);
+ if (status 0)
+ goto fail;
+
+ /* now we know the gpio is valid and chip won't vanish */
+
+ spin_unlock_irqrestore(gpio_lock, flags);
+
+ might_sleep_if(extra_checks chip-can_sleep);
+
+ return chip-set_debounce(chip, gpio, debounce);
+
+fail:
+ spin_unlock_irqrestore(gpio_lock, flags);
+ if (status)
+ pr_debug(%s: gpio-%d status %d\n,
+ __func__, gpio, status);
+
+ return status;
+}
+EXPORT_SYMBOL_GPL(gpio_set_debounce);
nitlet: I suspect this function is taking gpio_lock sooner than it
strictly needs to. Find-tuning that would decrease contention by an
insignificant amount ;)
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/6] teach gpiolib about gpio debouncing
On Mon, 17 May 2010 13:02:29 +0300 felipe.ba...@nokia.com wrote: From: Felipe Balbi felipe.ba...@nokia.com Hi all, I'm resending this series since no-one has had any further comments for quite some time. Adding Andrew Morton to the loop also since David Brownell didn't pick the patch neither comment to any version of it. David's been very quiet lately - please always cc me on gpio patches. When were these first sent? Has there been any feedback or external testing? I'm not seeing anything in the changelogs which helps me to understand how important this feature is and it's rather late to be merging 2.6.35 feature work.. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] gpiolib: introduce set_debounce method
On Thu, 20 May 2010 20:04:17 +0100 Alan Cox a...@lxorguk.ukuu.org.uk wrote: +EXPORT_SYMBOL_GPL(gpio_set_debounce); nitlet: I suspect this function is taking gpio_lock sooner than it strictly needs to. Find-tuning that would decrease contention by an insignificant amount ;) We need this for Intel MID boxes as well Andrew - so its a generic need. Thanks. I'm still wobbling on the 35-vs-36 line. Is that a big need or a little need? What user-visible problem does it solve, etc? Did anyone test it on the MID boxes? etc ;) -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc-omap: Add support for 16-bit and 32-bit registers
On Thu, 13 May 2010 12:29:35 -0700 Tony Lindgren t...@atomide.com wrote: * Cory Maccarrone darkstar6...@gmail.com [100307 09:44]: From: Marek Belisko marek.beli...@open-nandra.com The omap850 and omap730 use 16-bit registers instead of 32-bit, requiring a modification of the register addresses in the mmc-omap driver. To resolve this, a bit shift is performed on base register addresses, either by 1 or 2 bits depending on the CPU in use. This yields the correct registers for each CPU. Signed-off-by: Marek Belisko marek.beli...@open-nandra.com Signed-off-by: Cory Maccarrone darkstar6...@gmail.com Signed-off-by: Tony Lindgren t...@atomide.com Can you please queue this patch too? Ben's comments in this thread were for the i2c-omap, not for this MMC patch. We've had this patch in the omap tree for testing for quite a while now. The patch is also available in at: https://patchwork.kernel.org/patch/83971/ with the direct link to the mbox being: https://patchwork.kernel.org/patch/83971/mbox/ I already have that, as mmc-omap-add-support-for-16-bit-and-32-bit-registers.patch -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] omap_hsmmc: improve interrupt synchronisation
On Wed, 12 May 2010 10:50:45 +0300
Adrian Hunter adrian.hun...@nokia.com wrote:
From ad2e1cd024ccf9144b6620cfe808893719db738f Mon Sep 17 00:00:00 2001
From: Adrian Hunter adrian.hun...@nokia.com
Date: Wed, 14 Apr 2010 16:26:45 +0300
Subject: [PATCH] omap_hsmmc: improve interrupt synchronisation
The following changes were needed:
- do not use in_interrupt() because it will not work
with threaded interrupts
In addition, the following improvements were made:
- ensure DMA is unmapped only after the final DMA interrupt
- ensure a request is completed only after the final DMA interrupt
- disable controller interrupts when a request is not in progress
- remove the spin-lock protecting the start of a new request from
an unexpected interrupt because the locking was complicated and
a 'req_in_progress' flag suffices (since the spin-lock only defers
the unexpected interrupts anyway)
- instead use the spin-lock to protect the MMC interrupt handler
from the DMA interrupt handler
- remove the semaphore preventing DMA from being started while
the previous DMA is still in progress - the other changes make that
impossible, so it is now a BUG_ON condition
- ensure the controller interrupt status is clear before exiting
the interrrupt handler
In general, these changes make the code safer but do not fix any specific
bugs so backporting is not necessary.
...
+static void omap_hsmmc_request_done(struct omap_hsmmc_host *host, struct
mmc_request *mrq)
+{
+ int dma_ch;
+
+ spin_lock(host-irq_lock);
+ host-req_in_progress = 0;
+ dma_ch = host-dma_ch;
+ spin_unlock(host-irq_lock);
+
+ omap_hsmmc_disable_irq(host);
+ /* Do not complete the request if DMA is still in progress */
+ if (mrq-data host-use_dma dma_ch != -1)
+ return;
+ host-mrq = NULL;
+ mmc_request_done(host-mmc, mrq);
+}
Are we sure that irq_lock doesn't need to be taken in an irq-safe fashion?
send_init_stream() doesn't report an error if its busywait times out.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv5 0/3] Introduce the /proc/socinfo and use it to export OMAP data
On Tue, 11 May 2010 17:15:28 +0300 Eduardo Valentin eduardo.valen...@nokia.com wrote: Here is the version 5 of the change to export OMAP data to userspace (name, revision, id code, production id and die id). Basically, this version is still attempting to create a new file under /proc. It is the /proc/socinfo, which should be used to export bits which are SoC specific (not CPU related, nor machine related). So, differences between previous version are: - merged patch 02/04 with 03/04 to avoid compilation breakages. - simplified the seq_file usage by using the single_open and single_release functions - exported a function to register a seq_operation .show callback - adapted the changes accordingly As usual, comments are welcome. This changelog would be rather more useful if it was to show us some sample output from /proc/socinfo, perhaps accompanied with an explanation for people who aren't familar with this area of the kernel. I'd have thought that sysfs was an appropriate place for this info. Perhaps under /sys/devices/platform? Or /sys/devices/system? Peter's original patch didn't tell us where in the hierarchy the file was placed, nor why it was placed there, not what its contents look like. But crappy changelogs are the norm :( The objections stated in this email: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg17630.html appear to still apply to this version of the patches? Kevin didn't explain why he said Please export these via debugfs. Tony didn't clearly explain why he said I don't think we want to export unique chip identifiers by default. So apart from having certain opinions regarding communication skills and wondering why people cc me on stuff without vaguely providing enough info for me to understand what they're thinking, I don't know what to make of it all :( -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: regression: crash from 'ls /sys/modules/wl1251_spi/notes'
On Thu, 31 Dec 2009 22:15:08 +0100
Helge Deller del...@gmx.de wrote:
On 12/30/2009 04:49 PM, James Bottomley wrote:
A better, and more comprehensive patch would be to try not to count the
empty text sections when we're building the notes section (and actually
anywhere else in the file). This patch actually relies on the fact that
if sh_size is zero for the text section it should be for the
corresponding notes section. If that doesn't work, we'd actually have
to do the matching in the construction piece.
Can you try it to see if it works for you? If it doesn't, I'll try
matching notes to text. It works fine on parisc, but as we don't have a
notes section, that's not saying much ...
Thanks,
James
Ben Hutchings already sent a similar patch.
See: http://patchwork.kernel.org/patch/68925/
IMHO James patch below seems better since it
checks if a section will be allocated at a few more
places...
Ben's patch (which is below) is currently in linux-next, via a Rusty
tree. It is marked for -stable backporting.
If James's patch is preferable then there's an opportunity to do the
swap if we move promptly.
commit 9e9b48a89ed43c73d7355ff999b8e87b0628e1cd
Author: Ben Hutchings b...@decadent.org.uk
AuthorDate: Sat Dec 19 14:43:01 2009 +
Commit: Stephen Rothwell s...@canb.auug.org.au
CommitDate: Tue Jan 5 08:44:50 2010 +1100
modules: Skip empty sections when exporting section notes
Commit 35dead4 modules: don't export section names of empty sections
via sysfs changed the set of sections that have attributes, but did
not change the iteration over these attributes in add_notes_attrs().
This can lead to add_notes_attrs() creating attributes with the wrong
names or with null name pointers.
Introduce a sect_empty() function and use it in both add_sect_attrs()
and add_notes_attrs().
Reported-by: Martin Michlmayr t...@cyrius.com
Signed-off-by: Ben Hutchings b...@decadent.org.uk
Tested-by: Martin Michlmayr t...@cyrius.com
Cc: sta...@kernel.org
Signed-off-by: Rusty Russell ru...@rustcorp.com.au
diff --git a/kernel/module.c b/kernel/module.c
index e96b8ed..f82386b 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1010,6 +1010,12 @@ static const struct kernel_symbol
*resolve_symbol(Elf_Shdr *sechdrs,
* J. Corbet cor...@lwn.net
*/
#if defined(CONFIG_KALLSYMS) defined(CONFIG_SYSFS)
+
+static inline bool sect_empty(const Elf_Shdr *sect)
+{
+ return !(sect-sh_flags SHF_ALLOC) || sect-sh_size == 0;
+}
+
struct module_sect_attr
{
struct module_attribute mattr;
@@ -1051,8 +1057,7 @@ static void add_sect_attrs(struct module *mod, unsigned
int nsect,
/* Count loaded sections and allocate structures */
for (i = 0; i nsect; i++)
- if (sechdrs[i].sh_flags SHF_ALLOC
-sechdrs[i].sh_size)
+ if (!sect_empty(sechdrs[i]))
nloaded++;
size[0] = ALIGN(sizeof(*sect_attrs)
+ nloaded * sizeof(sect_attrs-attrs[0]),
@@ -1070,9 +1075,7 @@ static void add_sect_attrs(struct module *mod, unsigned
int nsect,
sattr = sect_attrs-attrs[0];
gattr = sect_attrs-grp.attrs[0];
for (i = 0; i nsect; i++) {
- if (! (sechdrs[i].sh_flags SHF_ALLOC))
- continue;
- if (!sechdrs[i].sh_size)
+ if (sect_empty(sechdrs[i]))
continue;
sattr-address = sechdrs[i].sh_addr;
sattr-name = kstrdup(secstrings + sechdrs[i].sh_name,
@@ -1156,7 +1159,7 @@ static void add_notes_attrs(struct module *mod, unsigned
int nsect,
/* Count notes sections and allocate structures. */
notes = 0;
for (i = 0; i nsect; i++)
- if ((sechdrs[i].sh_flags SHF_ALLOC)
+ if (!sect_empty(sechdrs[i])
(sechdrs[i].sh_type == SHT_NOTE))
++notes;
@@ -1172,7 +1175,7 @@ static void add_notes_attrs(struct module *mod, unsigned
int nsect,
notes_attrs-notes = notes;
nattr = notes_attrs-attrs[0];
for (loaded = i = 0; i nsect; ++i) {
- if (!(sechdrs[i].sh_flags SHF_ALLOC))
+ if (sect_empty(sechdrs[i]))
continue;
if (sechdrs[i].sh_type == SHT_NOTE) {
nattr-attr.name = mod-sect_attrs-attrs[loaded].name;
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] CPUidle: always return with interrupts enabled
On Fri, 16 Oct 2009 17:25:04 +0100 Martin Michlmayr t...@cyrius.com wrote: * Andrew Morton a...@linux-foundation.org [2009-10-06 13:34]: Rigor mortis is setting in on this one. The patch seems correct to me. Can someone put this patch in now? The problem has also been reported on Marvell's Kirkwood platform (ARM) by a number of users and the patch fixes it. Tested-by: Martin Michlmayr t...@cyrius.com I have it in my for-2.6.32 queue. Please CC stable when you commit the patch since it also needs to go in for 2.6.31. hm, I didn't know that. So noted, thanks. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] omap fixes for v2.6.32-rc3
On Fri, 9 Oct 2009 12:26:42 -0700 Greg KH gre...@suse.de wrote: What kind of issues? __Why would I be interested, where is the problem? The obvious-brain-dead-duh kind of issues, like this one. The problem is that OMAP devices (like beagleboard) are not booting correctly right now because of a wrong merge. It has been identified, tested, and acked, but nobody has picked it up for a pull request, so it's not clear it will be on -rc4. Has the patches been sent from the maintainer to Linus? Who is the maintainer? Who normally sends this stuff? To me it's not clear who should push the patch, it seems it doesn't belong on linux-omap, so Tony is pushing it through linux-mmc, which I don't think is the right place. They should be handling mmc-related issues, not obvious breakage. Well, mmc related breakage is fine to handle :) In order to keep the engines oiled I think there must be a process to flag obvious generic breakage so it's immediately picked; maybe an 'obvious-fixes' tree, or a 'simple-fixes' that has 'trivial', 'includecheck', and similar, or maybe nothing needs to be done. Up to you to decide. Andrew usually sends stuff like this at times. yeah, when in doubt, send it to me. When not in doubt send it to me anyway ;) I'll take care of bugging the appropriate maintainer or merging it directly. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] omap fixes for v2.6.32-rc3
On Fri, 9 Oct 2009 23:42:27 +0200 (CEST) Jiri Kosina jkos...@suse.cz wrote: On Fri, 9 Oct 2009, Greg KH wrote: There's a tree for trivial fixes: git://git.kernel.org/pub/scm/linux/kernel/git/jikos/trivial Well, as this is quite a high-priority functional fix, I'd suggest using other channels that trivial tree, as this one is primarily intended for simple fixes that are not really urgent. The problem is that OMAP devices (like beagleboard) are not booting correctly right now because of a wrong merge. It has been identified, tested, and acked, but nobody has picked it up for a pull request, so it's not clear it will be on -rc4. Has the patches been sent from the maintainer to Linus? Who is the maintainer? Who normally sends this stuff? The problem is that MMC now doesn't have proper maintainer, since Pierre stepped down from maintaining this stuff. Still, for the fix mentioned on http://www.mail-archive.com/linux-...@vger.kernel.org/msg00539.html the most appropriate contact would probably be Madhusudhan Chikkature. Adding him to CC of this thread. hm. What's all the fuss about? That patch was sent To:me and affects only MMC and is clearly a bugfix. I'll merge it when i get to it (hopefully tomorrow) then all is good? -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] CPUidle: always return with interrupts enabled
On Wed, 30 Sep 2009 23:21:33 +0200
Rafael J. Wysocki r...@sisk.pl wrote:
On Wednesday 30 September 2009, Kevin Hilman wrote:
In the case where cpuidle_idle_call() returns before changing state
due to a need_resched(), it was returning with IRQs disabled.
This patch ensures IRQs are (re)enabled before returning.
Venki, any comments on this?
Rigor mortis is setting in on this one.
Venki's most recent linux-acpi email was on July 31.
Reported-by: Hemanth V heman...@ti.com
Signed-off-by: Kevin Hilman khil...@deeprootsystems.com
---
drivers/cpuidle/cpuidle.c |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index ad41f19..12fdd39 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -76,8 +76,11 @@ static void cpuidle_idle_call(void)
#endif
/* ask the governor for the next state */
next_state = cpuidle_curr_governor-select(dev);
- if (need_resched())
+ if (need_resched()) {
+ local_irq_enable();
return;
+ }
+
target_state = dev-states[next_state];
/* enter the state and update stats */
The patch seems correct to me. The code is hopelessly poorly
documented as per usual, but other paths in that function, including
the call to target_state-enter() (which devolves to default_idle) also
enable interrupts.
Kevin, the changelog is not good. It tells us what was wrong with the
code but does not describe the user-visible effects of the bug.
I'm unable to find any bug report from Hemanth so I'm all in the dark.
Your cc to linux-omap makes me suspect that whatever the problem was
was exhibited on a non-x86 platform, under some circumstances. Perhaps
that explains (for unknown reasons) why whatever the problem was was
not observed on x86. But I'm totally in the dark and grasping for
clues and have no way of determining (for example) whether we should
backport the fix to earlier kernels.
Please send along the additional information?
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: OMAP2/3 Display Subsystem merging
On Wed, 23 Sep 2009 10:04:09 +0300 Tomi Valkeinen tomi.valkei...@nokia.com wrote: Hi, How can we proceed with the OMAP DSS2 merging? I can take the old omapfb patches you have to my tree, and give that tree to you as one set, or I can base the DSS2 on top of your tree. If that's ok, where can I find the patches or your tree? Wait for -rc1 (or tomorrow's mainline, probably), redo and retest the patches, send them out for review and integration. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL]: OMAP2/3 Display Subsystem
On Tue, 22 Sep 2009 15:29:48 +0300 Tomi Valkeinen tomi.valkei...@nokia.com wrote: Linus, Here's the new display subsystem and framebuffer driver for OMAP2/3. The driver has been reviewed on linux-omap and linux-fbdev-devel, and is in use, for example, on N900, Beagle Board and Overo boards. We have an ACK for the OMAP parts from Tony Lindgren, who maintains the OMAP platform. We have gotten positive feedback about the driver, for example in this thread: http://marc.info/?l=linux-kernelm=125171081901820w=2 The driver is actively maintained and developed further, and I have already a bunch of patches on top of these patches, but I would like to get the big core driver merged first. After the driver is merged, other people can send patches to LCD drivers and board files to enable the new display subsystem on their boards. Andrew said 2-3 weeks ago that he'll merge the driver (after he has looked through it), but he hasn't responded since then. I'm guessing he's quite busy. That's because it came very late and conflicts considerably with changes which are already pending. Please pull the new OMAP2/3 display subsystem driver from: git://gitorious.org/linux-omap-dss2/linux.git for-linus Confused. These conflicts heavily with the changes which I've already queued, does it not? Ones which were queued way earlier than this material. If so, why on earth did you send a pull request, knowing that it would trash my tree? -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V3 28/30] omap_hsmmc: ensure all clock enables and disables are paired
On Wed, 09 Sep 2009 15:00:03 +0300 Adrian Hunter adrian.hun...@nokia.com wrote: From baf6574a1b5e7c4fdc4a66d9e038efeee75ea1a0 Mon Sep 17 00:00:00 2001 From: Adrian Hunter adrian.hun...@nokia.com Date: Sun, 31 May 2009 19:27:36 +0300 Subject: [PATCH] omap_hsmmc: ensure all clock enables and disables are paired This patch hasn't been updated with omap_hsmmc-ensure-all-clock-enables-and-disables-are-paired-fix-for-the-db-clock-failure-message.patch, and omap_hsmmc-ensure-all-clock-enables-and-disables-are-paired-fix-for-the-db-clock-failure-message.patch is not present in the v3 patchset. What's up with that? I went through the patchset seeing what you'd done. mmc-add-host-capabilities-for-sd-only-and-mmc-only.patch and omap_hsmmc-pass-host-capabilities-for-sd-only-and-mmc-only.patch were dropped. arm-omap-rx51-set-mmc-capabilities-and-power-saving-flag.patch was significantly altered (I queued a delta for that). omap_hsmmc-make-use-of-new-mmc_cap_nonremovable-host-capability.patch was changed a bit to fix rejects (I think). -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] OMAP4: MMC driver support on OMAP4
On Wed, 2 Sep 2009 18:54:36 +0530 (IST) kishore kadiyala kishore.kadiy...@ti.com wrote: This Patch adds basic support for all 5 MMC controllers on OMAP4. -Kishore Signed-off-by: Kishore Kadiyala kishore.kadiy...@ti.com --- This patch doesn't include mmc-regulator support arch/arm/mach-omap2/devices.c | 42 + arch/arm/plat-omap/include/mach/irqs.h |2 + arch/arm/plat-omap/include/mach/mmc.h |9 ++- drivers/mmc/host/Kconfig |6 ++-- drivers/mmc/host/omap_hsmmc.c | 10 +++ 5 files changed, 60 insertions(+), 9 deletions(-) This looks like an ARM patch more than an MMC patch. I grabbed it as an MMC thing but hopefully someone else will grab it for a more appropriate tree. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/18] OMAP: DSS2: Intro
On Tue, 01 Sep 2009 10:10:19 +0300 Artem Bityutskiy dedeki...@gmail.com wrote: Andrew, could you please help with merging this piece of (well written) code? Could you give your blessing to include it into linux-next now, and merge this during the next merge window? I'll merge them (after I've looked through them, which I'll do now). But there are more rejects than I'm prepared to cope with. The various arch/arm files have undergone some changes in linux-next which yield more breakage than I'm prepared to try to fix. For example, arch/arm/mach-omap2/board-3430sdp.c:sdp3430_config[] ends up being an empty array! Then there's the matter of these patches, already in -mm: omapfb-add-support-for-the-apollon-lcd.patch omapfb-add-support-for-mipi-dcs-compatible-lcds.patch omapfb-add-support-for-the-amstrad-delta-lcd.patch omapfb-add-support-for-the-2430sdp-lcd.patch omapfb-add-support-for-the-omap2evm-lcd.patch omapfb-add-support-for-the-3430sdp-lcd.patch omapfb-add-support-for-the-omap3-evm-lcd.patch omapfb-add-support-for-the-omap3-beagle-dvi-output.patch omapfb-add-support-for-the-gumstix-overo-lcd.patch omapfb-add-support-for-the-zoom-mdk-lcd.patch omapfb-add-support-for-rotation-on-the-blizzard-lcd-ctrl.patch n770-enable-lcd-mipi-dcs-in-kconfig.patch omapfb-dispc-various-typo-fixes.patch omapfb-dispc-disable-iface-clocks-along-with-func-clocks.patch omapfb-dispc-enable-wake-up-capability.patch omapfb-dispc-allow-multiple-external-irq-handlers.patch omapfb-suspend-resume-only-if-fb-device-is-already-initialized.patch omapfb-fix-coding-style-remove-dead-line.patch omapfb-add-fb-manual-update-option-to-kconfig.patch omapfb-hwa742-fix-pointer-to-be-const.patch -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 0/32] mmc and omap_hsmmc patches
On Thu, 13 Aug 2009 10:27:15 -0500 Madhusudhan madhu...@ti.com wrote: I am curious to know is this series lined up for upstream? yup. http://userweb.kernel.org/~akpm/mmotm/ is the place to check for that. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 32/32] ARM: OMAP: RX51: set MMC capabilities and power-saving flag
On Mon, 27 Jul 2009 14:02:47 +0300 Adrian Hunter adrian.hun...@nokia.com wrote: Matt Fleming wrote: On Mon, Jul 27, 2009 at 10:15:11AM +0100, Matt Fleming wrote: On Mon, Jul 27, 2009 at 11:58:22AM +0300, Adrian Hunter wrote: Yes much nicer. Will you add it to your patch? Sure, I'll reply with an updated patch to 07/32 of this series. Thanks Adrian. OK, slight problem. If I change omap_hsmmc.c in 07/32 you'll need to respin all your patches from [PATCH 08/32] onwards. Is that OK with you? I'd like to know what Andrew wants to do, as these patches are in mm tree. In the past I have seen him ask for changes to be patched on top, rather than make a new patch. Otherwise I am happy to redo the patch set. Respinning the patchset produces the nicest result, but if it's just a matter of one modest fixup patch to be applied after the 32-patch series then I don't think it'll kill anyone. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 20/32] omap_hsmmc: put MMC regulator to sleep
On Fri, 10 Jul 2009 15:42:27 +0300
Adrian Hunter adrian.hun...@nokia.com wrote:
+static int omap_mmc_regsleep_to_enabled(struct mmc_omap_host *host)
+{
+ unsigned long timeout;
+
+ dev_dbg(mmc_dev(host-mmc), REGSLEEP - ENABLED\n);
+
+ clk_enable(host-fclk);
+ clk_enable(host-iclk);
+
+ if (clk_enable(host-dbclk))
+ dev_dbg(mmc_dev(host-mmc),
+ Enabling debounce clk failed\n);
+
+ omap_mmc_restore_ctx(host);
+
+ /*
+ * We turned off interrupts and bus power. Interrupts
+ * are turned on by 'mmc_omap_start_command()' so we
+ * just need to turn on the bus power here.
+ */
+ OMAP_HSMMC_WRITE(host-base, HCTL,
+ OMAP_HSMMC_READ(host-base, HCTL) | SDBP);
+
+ timeout = jiffies + msecs_to_jiffies(MMC_TIMEOUT_MS);
+ while ((OMAP_HSMMC_READ(host-base, HCTL) SDBP) != SDBP
+time_before(jiffies, timeout))
+ ;
+
+ if (mmc_slot(host).set_sleep)
+ mmc_slot(host).set_sleep(host-dev, host-slot_id,
+ 0, host-vdd, 0);
+
+ host-dpm_state = ENABLED;
+
+ return 0;
+}
We take no action if the wait for SDBP timed out?
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 26/32] omap_hsmmc: prevent races with irq handler
On Fri, 10 Jul 2009 15:43:09 +0300 Adrian Hunter adrian.hun...@nokia.com wrote: From 242fae6293adec671b14354f215217354f5076a0 Mon Sep 17 00:00:00 2001 From: Adrian Hunter adrian.hun...@nokia.com Date: Sat, 16 May 2009 10:32:34 +0300 Subject: [PATCH] omap_hsmmc: prevent races with irq handler If an unexpected interrupt occurs while preparing the next request, an oops can occur. For example, a new request is setting up DMA for data transfer so host-data is not NULL. An unexpected transfer complete (TC) interrupt comes along and the interrupt handler sets host-data to NULL. Oops! Prevent that by disabling interrupts while setting up a new request. Signed-off-by: Adrian Hunter adrian.hun...@nokia.com --- drivers/mmc/host/omap_hsmmc.c | 16 1 files changed, 16 insertions(+), 0 deletions(-) diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c index 28563d6..38e1410 100644 --- a/drivers/mmc/host/omap_hsmmc.c +++ b/drivers/mmc/host/omap_hsmmc.c @@ -452,6 +452,13 @@ mmc_omap_start_command(struct mmc_omap_host *host, struct mmc_command *cmd, if (host-use_dma) cmdreg |= DMA_EN; + /* + * In an interrupt context (i.e. STOP command), the interrupt is already + * enabled, otherwise it is not (i.e. new request). + */ + if (!in_interrupt()) + enable_irq(host-irq); + OMAP_HSMMC_WRITE(host-base, ARG, cmd-arg); OMAP_HSMMC_WRITE(host-base, CMD, cmdreg); } @@ -1011,6 +1018,13 @@ static void omap_mmc_request(struct mmc_host *mmc, struct mmc_request *req) struct mmc_omap_host *host = mmc_priv(mmc); int err; + /* + * Prevent races with the interrupt handler because of unexpected + * interrupts, but not if we are already in interrupt context i.e. + * retries. + */ + if (!in_interrupt()) + disable_irq(host-irq); WARN_ON(host-mrq != NULL); host-mrq = req; err = mmc_omap_prepare_data(host, req); @@ -1019,6 +1033,8 @@ static void omap_mmc_request(struct mmc_host *mmc, struct mmc_request *req) if (req-data) req-data-error = err; host-mrq = NULL; + if (!in_interrupt()) + enable_irq(host-irq); mmc_request_done(mmc, req); return; } That seems pretty crude. Disabling an interrupt line can be expensive, and will shut off any other innocent devices which share the line. The usual and superior way of fixing races such as this is spin_lock_irq[save](). -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Linux-fbdev-devel] [PATCH] omapfb: Fix argument of blank operation.
On Wed, 11 Mar 2009 22:47:41 +0530 Trilok Soni soni.tri...@gmail.com wrote: Hi Felipe, On Fri, Dec 5, 2008 at 4:15 AM, Felipe Contreras I bet he thought we'd forgotten. felipe.contre...@gmail.com wrote: From: Felipe Contreras felipe.contre...@nokia.com The blank operation should receive FB_BLANK_POWERDOWN, not VESA_POWERDOWN. Thanks. Looks good. Signed-off-by: Trilok Soni soni.tri...@gmail.com Unfortunately the changelog didn't give me any hint as to the seriousness of the problem which was fixed. So I queued it for 2.6.30, perhaps inappropriately. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] OMAP: MMC: recover from transfer failures - Resend
On Tue, 3 Feb 2009 15:05:58 +0100 Jean Pihet jpi...@mvista.com wrote: + while (OMAP_HSMMC_READ(host-base, + SYSCTL) SRD) + ; Is a __raw_readl() sufficient to prevent the cpu from burning up here, or should we add cpu_relax()? An infinite loop which assumes the hardware is perfect is always a worry. But I see the driver already does that, so we're no worse off.. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] HDQ Driver for OMAP2430/3430
On Tue, 14 Oct 2008 14:21:50 +0530 Madhusudhan Chikkature [EMAIL
PROTECTED] wrote:
- Original Message -
From: Andrew Morton [EMAIL PROTECTED]
To: Madhusudhan Chikkature [EMAIL PROTECTED]
Cc: [EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED];
linux-omap@vger.kernel.org
Sent: Monday, October 13, 2008 9:23 PM
Subject: Re: [PATCH 1/5] HDQ Driver for OMAP2430/3430
On Mon, 13 Oct 2008 18:55:43 +0530 Madhusudhan Chikkature [EMAIL
PROTECTED] wrote:
- Original Message -
From: Andrew Morton [EMAIL PROTECTED]
To: Gadiyar, Anand [EMAIL PROTECTED]
Cc: [EMAIL PROTECTED]; [EMAIL PROTECTED];
linux-omap@vger.kernel.org; [EMAIL PROTECTED]
Sent: Saturday, October 11, 2008 2:08 AM
Subject: Re: [PATCH 1/5] HDQ Driver for OMAP2430/3430
+ /* set the GO bit */
+ hdq_reg_merge(hdq_data, OMAP_HDQ_CTRL_STATUS,
OMAP_HDQ_CTRL_STATUS_GO,
+ OMAP_HDQ_CTRL_STATUS_DIR | OMAP_HDQ_CTRL_STATUS_GO);
+ /* wait for the TXCOMPLETE bit */
+ ret = wait_event_interruptible_timeout(hdq_wait_queue,
+ hdq_data-hdq_irqstatus, OMAP_HDQ_TIMEOUT);
+ if (ret 0) {
+ dev_dbg(hdq_data-dev, wait interrupted);
+ return -EINTR;
+ }
Is this desirable? The user hits ^C and the driver bails out?
I assume so, but was this tested?
Andrew, What is the test scenario you mean here? A user hitting ^C when
the driver is waiting for the TXCOMPLETE bit?
Yes.
Yes. It is desired to return an error if the condition in the wait is not
met. I need to change the check for return value to check for zero and neg
value.
I spent some time to test this perticular scenario.I could not really see any
impact of hitting ^C when an application is
transfering data in the background. When the h/w is programmed to transfer
data and the driver issues a wait, I see that
TXCOMPLETE interrupt comes immediately and wakes up as expected.
So guess I am unable to hit ^C exactly when the driver is waiting in
wait_event_interruptible_timeout(before the condition
is met) for it to catch the signal. Is it generally suggested to use
wait_event_timeout so that ^C signal is not caught?
I think it's reasonable to permit the driver's operations to be interrupted
in this manner. It's done in quite a few other places. But the problem is
actually *testing* it.
I guess one could do a whitebox-style test. Add new code like:
{
static int x;
if (!(x++ % 1000)) {
printk(hit ^c now\n);
msleep(2000);
}
}
in the right place.
Tricky.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] HDQ Driver for OMAP2430/3430
On Tue, 14 Oct 2008 17:42:49 +0400 Evgeniy Polyakov [EMAIL PROTECTED] wrote: Hi. On Tue, Oct 14, 2008 at 05:50:02AM -0700, Andrew Morton ([EMAIL PROTECTED]) wrote: I think it's reasonable to permit the driver's operations to be interrupted in this manner. It's done in quite a few other places. But the problem is actually *testing* it. Why not just skipping the waiting and returning error pretending user really sent a signal? Better than nothing, but because signal_pending() isn't actually true, upper layers wil behave differently. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] HDQ Driver for OMAP2430/3430
On Mon, 13 Oct 2008 18:55:43 +0530 Madhusudhan Chikkature [EMAIL
PROTECTED] wrote:
- Original Message -
From: Andrew Morton [EMAIL PROTECTED]
To: Gadiyar, Anand [EMAIL PROTECTED]
Cc: [EMAIL PROTECTED]; [EMAIL PROTECTED]; linux-omap@vger.kernel.org;
[EMAIL PROTECTED]
Sent: Saturday, October 11, 2008 2:08 AM
Subject: Re: [PATCH 1/5] HDQ Driver for OMAP2430/3430
+ /* set the GO bit */
+ hdq_reg_merge(hdq_data, OMAP_HDQ_CTRL_STATUS,
OMAP_HDQ_CTRL_STATUS_GO,
+ OMAP_HDQ_CTRL_STATUS_DIR | OMAP_HDQ_CTRL_STATUS_GO);
+ /* wait for the TXCOMPLETE bit */
+ ret = wait_event_interruptible_timeout(hdq_wait_queue,
+ hdq_data-hdq_irqstatus, OMAP_HDQ_TIMEOUT);
+ if (ret 0) {
+ dev_dbg(hdq_data-dev, wait interrupted);
+ return -EINTR;
+ }
Is this desirable? The user hits ^C and the driver bails out?
I assume so, but was this tested?
Andrew, What is the test scenario you mean here? A user hitting ^C when the
driver is waiting for the TXCOMPLETE bit?
Yes.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] HDQ Driver for OMAP2430/3430
On Wed, 8 Oct 2008 12:46:25 +0530
Gadiyar, Anand [EMAIL PROTECTED] wrote:
From: Madhusudhan Chikkature [EMAIL PROTECTED]
The HDQ/1-Wire module of TI OMAP2430/3430 platforms implement the hardware
protocol of the master functions of the Benchmark HDQ and the Dallas
Semiconductor 1-Wire protocols. These protocols use a single wire for
communication between the master (HDQ/1-Wire controller) and the slave
(HDQ/1-Wire external compliant device).
This patch provides the HDQ driver to suppport TI OMAP2430/3430 platforms.
Every tab character in all five patches was converted to eight-spaces by
your email client. Please fix the mailer and resend everything.
+++ linux-2.6/drivers/w1/masters/omap_hdq.c 2008-09-26 14:28:36.0
+0530
@@ -0,0 +1,730 @@
+/*
+ * drivers/w1/masters/omap_hdq.c
+ *
+ * Copyright (C) 2007 Texas Instruments, Inc.
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2. This program is licensed as is without any warranty of any
+ * kind, whether express or implied.
+ *
+ */
+#include linux/kernel.h
+#include linux/module.h
+#include linux/platform_device.h
+#include linux/interrupt.h
+#include linux/err.h
+#include linux/clk.h
+#include linux/io.h
+#include asm/irq.h
+#include mach/hardware.h
We conventionally put a blank line between the linux/ includes and the
asm/ includes.
+static int omap_hdq_get(struct hdq_data *hdq_data);
+static int omap_hdq_put(struct hdq_data *hdq_data);
+static int omap_hdq_break(struct hdq_data *hdq_data);
These three aren't strictly needed, because these functions are defined
before first use. I think it's best to not declare them.
+static int hdq_wait_for_flag(struct hdq_data *hdq_data, u32 offset,
+ u8 flag, u8 flag_set, u8 *status)
+{
+ int ret = 0;
+ unsigned long timeout = jiffies + OMAP_HDQ_TIMEOUT;
+
+ if (flag_set == OMAP_HDQ_FLAG_CLEAR) {
+ /* wait for the flag clear */
+ while (((*status = hdq_reg_in(hdq_data, offset)) flag)
+time_before(jiffies, timeout)) {
+ set_current_state(TASK_UNINTERRUPTIBLE);
+ schedule_timeout(1);
Use schedule_timeout_uninterruptible(1)
+ }
+ if (*status flag)
+ ret = -ETIMEDOUT;
+ } else if (flag_set == OMAP_HDQ_FLAG_SET) {
+ /* wait for the flag set */
+ while (!((*status = hdq_reg_in(hdq_data, offset)) flag)
+time_before(jiffies, timeout)) {
+ set_current_state(TASK_UNINTERRUPTIBLE);
+ schedule_timeout(1);
elsewhere..
+ }
+ if (!(*status flag))
+ ret = -ETIMEDOUT;
+ } else
+ return -EINVAL;
+
+ return ret;
+}
+
+/* write out a byte and fill *status with HDQ_INT_STATUS */
+static int hdq_write_byte(struct hdq_data *hdq_data, u8 val, u8 *status)
+{
+ int ret;
+ u8 tmp_status;
+ unsigned long irqflags;
+
+ *status = 0;
+
+ spin_lock_irqsave(hdq_data-hdq_spinlock, irqflags);
+ /* clear interrupt flags via a dummy read */
+ hdq_reg_in(hdq_data, OMAP_HDQ_INT_STATUS);
+ /* ISR loads it with new INT_STATUS */
+ hdq_data-hdq_irqstatus = 0;
+ spin_unlock_irqrestore(hdq_data-hdq_spinlock, irqflags);
+
+ hdq_reg_out(hdq_data, OMAP_HDQ_TX_DATA, val);
+
+ /* set the GO bit */
+ hdq_reg_merge(hdq_data, OMAP_HDQ_CTRL_STATUS, OMAP_HDQ_CTRL_STATUS_GO,
+ OMAP_HDQ_CTRL_STATUS_DIR | OMAP_HDQ_CTRL_STATUS_GO);
+ /* wait for the TXCOMPLETE bit */
+ ret = wait_event_interruptible_timeout(hdq_wait_queue,
+ hdq_data-hdq_irqstatus, OMAP_HDQ_TIMEOUT);
+ if (ret 0) {
+ dev_dbg(hdq_data-dev, wait interrupted);
+ return -EINTR;
+ }
Is this desirable? The user hits ^C and the driver bails out?
I assume so, but was this tested?
+ spin_lock_irqsave(hdq_data-hdq_spinlock, irqflags);
+ *status = hdq_data-hdq_irqstatus;
+ spin_unlock_irqrestore(hdq_data-hdq_spinlock, irqflags);
It's unusual to put a lock around a single atomic move instruction.
+ /* check irqstatus */
+ if (!(*status OMAP_HDQ_INT_STATUS_TXCOMPLETE)) {
+ dev_dbg(hdq_data-dev, timeout waiting for
+ TXCOMPLETE/RXCOMPLETE, %x, *status);
+ return -ETIMEDOUT;
+ }
+
+ /* wait for the GO bit return to zero */
+ ret = hdq_wait_for_flag(hdq_data, OMAP_HDQ_CTRL_STATUS,
+ OMAP_HDQ_CTRL_STATUS_GO,
+ OMAP_HDQ_FLAG_CLEAR, tmp_status);
+ if (ret) {
+ dev_dbg(hdq_data-dev, timeout waiting GO bit
+ return to zero, %x, tmp_status);
+
