[PATCH] MAINTAINERS: Add maintainer for maxim codecs

[Anish Kumar]

This patch adds maintainer for maxim audio codecs.
Signed-off-by: Anish Kumar 
---
 MAINTAINERS | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8bdd7a7..2128586 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7387,6 +7387,13 @@ S:   Supported
 F: sound/soc/
 F: include/sound/soc*
 
+MAXIM INTEGRATED SOC CODEC DRIVER
+M: Anish Kumar 
+L: alsa-de...@alsa-project.org (moderated for non-subscribers)
+S: Maintained
+F: sound/soc/codecs/max*
+F: include/sound/max*
+
 SPARC + UltraSPARC (sparc/sparc64)
 M: "David S. Miller" 
 L: sparcli...@vger.kernel.org
-- 
1.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] MAINTAINERS: Add maintainer for maxim codecs

[Anish Kumar]

This patch adds maintainer for maxim audio codecs.
Signed-off-by: Anish Kumar yesanishh...@gmail.com
---
 MAINTAINERS | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8bdd7a7..2128586 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7387,6 +7387,13 @@ S:   Supported
 F: sound/soc/
 F: include/sound/soc*
 
+MAXIM INTEGRATED SOC CODEC DRIVER
+M: Anish Kumar yesanishh...@gmail.com
+L: alsa-de...@alsa-project.org (moderated for non-subscribers)
+S: Maintained
+F: sound/soc/codecs/max*
+F: include/sound/max*
+
 SPARC + UltraSPARC (sparc/sparc64)
 M: David S. Miller da...@davemloft.net
 L: sparcli...@vger.kernel.org
-- 
1.9.3

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [RFC]Watchdog:core: constant pinging until userspace timesout when delay very less

[anish kumar]

Certain watchdog drivers use a timer to keep kicking the watchdog at
a rate of 0.5s (HZ/2) untill userspace times out.They do this as
we can't guarantee that watchdog will be pinged fast enough
for all system loads, especially if timeout is configured for
less than or equal to 1 second(basically small values).

As suggested by Wim Van Sebroeck & Guenter Roeck we should
add this functionality of individual watchdog drivers in the core
watchdog core.

Signed-off-by: anish kumar 
---
 drivers/watchdog/watchdog_dev.c |   34 +-
 include/linux/watchdog.h|1 +
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index faf4e18..0305803 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -41,9 +41,14 @@
 #include   /* For handling misc devices */
 #include /* For __init/__exit/... */
 #include  /* For copy_to_user/put_user/... */
+#include 
+#include 
 
 #include "watchdog_core.h"
 
+/* Timer heartbeat (500ms) */
+#define WDT_TIMEOUT(HZ/2) /* should this be sysfs? */
+
 /* the dev_t structure to store the dynamically allocated watchdog devices */
 static dev_t watchdog_devt;
 /* the watchdog device behind /dev/watchdog */
@@ -73,16 +78,33 @@ static int watchdog_ping(struct watchdog_device *wddev)
if (!watchdog_active(wddev))
goto out_ping;
 
-   if (wddev->ops->ping)
-   err = wddev->ops->ping(wddev);  /* ping the watchdog */
-   else
-   err = wddev->ops->start(wddev); /* restart watchdog */
+   /* should we check ping interval value i.e. timeout value
+  if it is less than certain threshold then only we 
+  should add this logic of periodic pinging? */
+   if (time_before(jiffies, (unsigned long)wddev->timeout)) {
+   if (wddev->ops->ping)
+   err = wddev->ops->ping(wddev);/* ping the watchdog */
+   else
+   err = wddev->ops->start(wddev);/* restart watchdog */
+   mod_timer(>timer, jiffies + WDT_TIMEOUT);
+   } else {
+   /*
+*what we should when we find out that userspace
+*has timed out?
+**/
+   }
 
 out_ping:
mutex_unlock(>lock);
return err;
 }
 
+static void watchdog_ping_wrapper(unsigned long priv)
+{
+   struct watchdog_device *wdd = (void *)priv;
+   watchdog_ping(wdd);
+}
+
 /*
  * watchdog_start: wrapper to start the watchdog.
  * @wddev: the watchdog device to start
@@ -109,7 +131,8 @@ static int watchdog_start(struct watchdog_device *wddev)
err = wddev->ops->start(wddev);
if (err == 0)
set_bit(WDOG_ACTIVE, >status);
-
+   
+   mod_timer(>timer, jiffies + WDT_TIMEOUT);
 out_start:
mutex_unlock(>lock);
return err;
@@ -552,6 +575,7 @@ int watchdog_dev_register(struct watchdog_device *watchdog)
old_wdd = NULL;
}
}
+   setup_timer(>timer, 0, (long)watchdog_ping_wrapper);
return err;
 }
 
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 2a3038e..e5f18f7 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -84,6 +84,7 @@ struct watchdog_device {
const struct watchdog_ops *ops;
unsigned int bootstatus;
unsigned int timeout;
+   struct timer_list timer;
unsigned int min_timeout;
unsigned int max_timeout;
void *driver_data;
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [RFC]Watchdog:core: constant pinging until userspace timesout when delay very less

[anish kumar]

Certain watchdog drivers use a timer to keep kicking the watchdog at
a rate of 0.5s (HZ/2) untill userspace times out.They do this as
we can't guarantee that watchdog will be pinged fast enough
for all system loads, especially if timeout is configured for
less than or equal to 1 second(basically small values).

As suggested by Wim Van Sebroeck  Guenter Roeck we should
add this functionality of individual watchdog drivers in the core
watchdog core.

Signed-off-by: anish kumar anish198519851...@gmail.com
---
 drivers/watchdog/watchdog_dev.c |   34 +-
 include/linux/watchdog.h|1 +
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index faf4e18..0305803 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -41,9 +41,14 @@
 #include linux/miscdevice.h  /* For handling misc devices */
 #include linux/init.h/* For __init/__exit/... */
 #include linux/uaccess.h /* For copy_to_user/put_user/... */
+#include linux/timer.h
+#include linux/jiffies.h
 
 #include watchdog_core.h
 
+/* Timer heartbeat (500ms) */
+#define WDT_TIMEOUT(HZ/2) /* should this be sysfs? */
+
 /* the dev_t structure to store the dynamically allocated watchdog devices */
 static dev_t watchdog_devt;
 /* the watchdog device behind /dev/watchdog */
@@ -73,16 +78,33 @@ static int watchdog_ping(struct watchdog_device *wddev)
if (!watchdog_active(wddev))
goto out_ping;
 
-   if (wddev-ops-ping)
-   err = wddev-ops-ping(wddev);  /* ping the watchdog */
-   else
-   err = wddev-ops-start(wddev); /* restart watchdog */
+   /* should we check ping interval value i.e. timeout value
+  if it is less than certain threshold then only we 
+  should add this logic of periodic pinging? */
+   if (time_before(jiffies, (unsigned long)wddev-timeout)) {
+   if (wddev-ops-ping)
+   err = wddev-ops-ping(wddev);/* ping the watchdog */
+   else
+   err = wddev-ops-start(wddev);/* restart watchdog */
+   mod_timer(wddev-timer, jiffies + WDT_TIMEOUT);
+   } else {
+   /*
+*what we should when we find out that userspace
+*has timed out?
+**/
+   }
 
 out_ping:
mutex_unlock(wddev-lock);
return err;
 }
 
+static void watchdog_ping_wrapper(unsigned long priv)
+{
+   struct watchdog_device *wdd = (void *)priv;
+   watchdog_ping(wdd);
+}
+
 /*
  * watchdog_start: wrapper to start the watchdog.
  * @wddev: the watchdog device to start
@@ -109,7 +131,8 @@ static int watchdog_start(struct watchdog_device *wddev)
err = wddev-ops-start(wddev);
if (err == 0)
set_bit(WDOG_ACTIVE, wddev-status);
-
+   
+   mod_timer(wddev-timer, jiffies + WDT_TIMEOUT);
 out_start:
mutex_unlock(wddev-lock);
return err;
@@ -552,6 +575,7 @@ int watchdog_dev_register(struct watchdog_device *watchdog)
old_wdd = NULL;
}
}
+   setup_timer(watchdog-timer, 0, (long)watchdog_ping_wrapper);
return err;
 }
 
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 2a3038e..e5f18f7 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -84,6 +84,7 @@ struct watchdog_device {
const struct watchdog_ops *ops;
unsigned int bootstatus;
unsigned int timeout;
+   struct timer_list timer;
unsigned int min_timeout;
unsigned int max_timeout;
void *driver_data;
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[tip:perf/core] watchdog: Add comments to explain the watchdog_disabled variable

[tip-bot for anish kumar]

Commit-ID:  b66a2356d7108a15b8b5c9b8e6213e05ead22cd6
Gitweb: http://git.kernel.org/tip/b66a2356d7108a15b8b5c9b8e6213e05ead22cd6
Author: anish kumar 
AuthorDate: Tue, 12 Mar 2013 14:44:08 -0400
Committer:  Ingo Molnar 
CommitDate: Thu, 14 Mar 2013 08:24:05 +0100

watchdog: Add comments to explain the watchdog_disabled variable

The watchdog_disabled flag is a bit cryptic. However it's
usefulness is multifold. Uses are:

 1. Check if smpboot_register_percpu_thread function passed.

 2. Makes sure that user enables and disables the watchdog in
sequence i.e. enable watchdog->disable watchdog->enable watchdog
Unlike enable watchdog->enable watchdog which is wrong.

Signed-off-by: anish kumar 
[small text cleanups]
Signed-off-by: Don Zickus 
Cc: chuansheng@intel.com
Cc: paul...@linux.vnet.ibm.com
Link: 
http://lkml.kernel.org/r/1363113848-18344-1-git-send-email-dzic...@redhat.com
Signed-off-by: Ingo Molnar 
---
 kernel/watchdog.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 4a94467..05039e3 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -517,6 +517,11 @@ int proc_dowatchdog(struct ctl_table *table, int write,
return ret;
 
set_sample_period();
+   /*
+* Watchdog threads shouldn't be enabled if they are
+* disabled. The 'watchdog_disabled' variable check in
+* watchdog_*_all_cpus() function takes care of this.
+*/
if (watchdog_enabled && watchdog_thresh)
watchdog_enable_all_cpus();
else
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[tip:perf/core] watchdog: Add comments to explain the watchdog_disabled variable

[tip-bot for anish kumar]

Commit-ID:  b66a2356d7108a15b8b5c9b8e6213e05ead22cd6
Gitweb: http://git.kernel.org/tip/b66a2356d7108a15b8b5c9b8e6213e05ead22cd6
Author: anish kumar anish198519851...@gmail.com
AuthorDate: Tue, 12 Mar 2013 14:44:08 -0400
Committer:  Ingo Molnar mi...@kernel.org
CommitDate: Thu, 14 Mar 2013 08:24:05 +0100

watchdog: Add comments to explain the watchdog_disabled variable

The watchdog_disabled flag is a bit cryptic. However it's
usefulness is multifold. Uses are:

 1. Check if smpboot_register_percpu_thread function passed.

 2. Makes sure that user enables and disables the watchdog in
sequence i.e. enable watchdog-disable watchdog-enable watchdog
Unlike enable watchdog-enable watchdog which is wrong.

Signed-off-by: anish kumar anish198519851...@gmail.com
[small text cleanups]
Signed-off-by: Don Zickus dzic...@redhat.com
Cc: chuansheng@intel.com
Cc: paul...@linux.vnet.ibm.com
Link: 
http://lkml.kernel.org/r/1363113848-18344-1-git-send-email-dzic...@redhat.com
Signed-off-by: Ingo Molnar mi...@kernel.org
---
 kernel/watchdog.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 4a94467..05039e3 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -517,6 +517,11 @@ int proc_dowatchdog(struct ctl_table *table, int write,
return ret;
 
set_sample_period();
+   /*
+* Watchdog threads shouldn't be enabled if they are
+* disabled. The 'watchdog_disabled' variable check in
+* watchdog_*_all_cpus() function takes care of this.
+*/
if (watchdog_enabled  watchdog_thresh)
watchdog_enable_all_cpus();
else
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [Timer][Trivial] __clocksource_register_scale return value use?

[anish kumar]

__clocksource_register_scale() currently returns int but it should
return void as there are no error paths in that function.
Making it void would help some amount of code to be removed at various
places.

clocksource_register_hz/khz() return value is checked
in most of the places but I think it will translate to always
if(true) so let's remove those checks as well(patch will be sent
later for that).

Is this return value for some future usecase(?), if yes then my
apologies.

Signed-off-by: anish kumar 
---
 include/linux/clocksource.h |6 +++---
 kernel/time/clocksource.c   |7 +--
 2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 27cfda4..2b074cc 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -294,17 +294,17 @@ clocks_calc_mult_shift(u32 *mult, u32 *shift, u32
from, u32 to, u32 minsec);
  * Don't call __clocksource_register_scale directly, use
  * clocksource_register_hz/khz
  */
-extern int
+extern void
 __clocksource_register_scale(struct clocksource *cs, u32 scale, u32
freq);
 extern void
 __clocksource_updatefreq_scale(struct clocksource *cs, u32 scale, u32
freq);
 
-static inline int clocksource_register_hz(struct clocksource *cs, u32
hz)
+static inline void clocksource_register_hz(struct clocksource *cs, u32
hz)
 {
return __clocksource_register_scale(cs, 1, hz);
 }
 
-static inline int clocksource_register_khz(struct clocksource *cs, u32
khz)
+static inline void clocksource_register_khz(struct clocksource *cs, u32
khz)
 {
return __clocksource_register_scale(cs, 1000, khz);
 }
diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index c958338..1915550 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -703,14 +703,11 @@ EXPORT_SYMBOL_GPL(__clocksource_updatefreq_scale);
  * @scale: Scale factor multiplied against freq to get clocksource hz
  * @freq:  clocksource frequency (cycles per second) divided by scale
  *
- * Returns -EBUSY if registration fails, zero otherwise.
- *
  * This *SHOULD NOT* be called directly! Please use the
  * clocksource_register_hz() or clocksource_register_khz helper
functions.
  */
-int __clocksource_register_scale(struct clocksource *cs, u32 scale, u32
freq)
+void __clocksource_register_scale(struct clocksource *cs, u32 scale,
u32 freq)
 {
-
/* Initialize mult/shift and max_idle_ns */
__clocksource_updatefreq_scale(cs, scale, freq);
 
@@ -720,11 +717,9 @@ int __clocksource_register_scale(struct clocksource
*cs, u32 scale, u32 freq)
clocksource_enqueue_watchdog(cs);
clocksource_select();
mutex_unlock(_mutex);
-   return 0;
 }
 EXPORT_SYMBOL_GPL(__clocksource_register_scale);
 
-
 /**
  * clocksource_register - Used to install new clocksources
  * @cs:clocksource to be registered
-- 
1.7.1


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [Timer][Trivial] __clocksource_register_scale return value use?

[anish kumar]

__clocksource_register_scale() currently returns int but it should
return void as there are no error paths in that function.
Making it void would help some amount of code to be removed at various
places.

clocksource_register_hz/khz() return value is checked
in most of the places but I think it will translate to always
if(true) so let's remove those checks as well(patch will be sent
later for that).

Is this return value for some future usecase(?), if yes then my
apologies.

Signed-off-by: anish kumar anish198519851...@gmail.com
---
 include/linux/clocksource.h |6 +++---
 kernel/time/clocksource.c   |7 +--
 2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 27cfda4..2b074cc 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -294,17 +294,17 @@ clocks_calc_mult_shift(u32 *mult, u32 *shift, u32
from, u32 to, u32 minsec);
  * Don't call __clocksource_register_scale directly, use
  * clocksource_register_hz/khz
  */
-extern int
+extern void
 __clocksource_register_scale(struct clocksource *cs, u32 scale, u32
freq);
 extern void
 __clocksource_updatefreq_scale(struct clocksource *cs, u32 scale, u32
freq);
 
-static inline int clocksource_register_hz(struct clocksource *cs, u32
hz)
+static inline void clocksource_register_hz(struct clocksource *cs, u32
hz)
 {
return __clocksource_register_scale(cs, 1, hz);
 }
 
-static inline int clocksource_register_khz(struct clocksource *cs, u32
khz)
+static inline void clocksource_register_khz(struct clocksource *cs, u32
khz)
 {
return __clocksource_register_scale(cs, 1000, khz);
 }
diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index c958338..1915550 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -703,14 +703,11 @@ EXPORT_SYMBOL_GPL(__clocksource_updatefreq_scale);
  * @scale: Scale factor multiplied against freq to get clocksource hz
  * @freq:  clocksource frequency (cycles per second) divided by scale
  *
- * Returns -EBUSY if registration fails, zero otherwise.
- *
  * This *SHOULD NOT* be called directly! Please use the
  * clocksource_register_hz() or clocksource_register_khz helper
functions.
  */
-int __clocksource_register_scale(struct clocksource *cs, u32 scale, u32
freq)
+void __clocksource_register_scale(struct clocksource *cs, u32 scale,
u32 freq)
 {
-
/* Initialize mult/shift and max_idle_ns */
__clocksource_updatefreq_scale(cs, scale, freq);
 
@@ -720,11 +717,9 @@ int __clocksource_register_scale(struct clocksource
*cs, u32 scale, u32 freq)
clocksource_enqueue_watchdog(cs);
clocksource_select();
mutex_unlock(clocksource_mutex);
-   return 0;
 }
 EXPORT_SYMBOL_GPL(__clocksource_register_scale);
 
-
 /**
  * clocksource_register - Used to install new clocksources
  * @cs:clocksource to be registered
-- 
1.7.1


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Fwd: mmc card probe not getting called

[anish kumar]

On Wed, 2013-02-20 at 11:25 +0530, chetan cr123 wrote:
Avoiding top posting.
> Hi Anish,
> 
> Thanks for your reply,
> 
> I was doing device registration for device by giving same name as
> driver name, This i used to do in platform driver registration,
> 
> But i dont know how to do for mmc device registration,
> 
> And i also want to know which part of the code(file name) is doing the
> string compare with the driver and device names and calling the probe
> function. can u please point me to that part of code. from many days i
> was searching from which part of code where string compare is done and
> calls the probe function.
> 
> 
> Kindly point me out to that part of code.
look at drivers/base/dd.c
static int really_probe(struct device *dev, struct device_driver *drv)
{
//snip
if (dev->bus->probe) {
ret = dev->bus->probe(dev);
if (ret)
goto probe_failed;
} else if (drv->probe) {
ret = drv->probe(dev);
if (ret)
goto probe_failed;
}
Tip:Whenever you want to see how some function is being called use
dump_stack().This will give you the call chain leading up to your
function call which you are interested in.
> 
> 
> 
> On Tue, Feb 19, 2013 at 9:25 PM, anish kumar
>  wrote:
> > On Tue, 2013-02-19 at 12:16 +0530, chetan cr123 wrote:
> >> HI All,
> >>
> >> I am working on Sd Card/Block driver
> >>
> >> I am registering it as both
> >>
> >> 1. register_blkdev()-  BLOCK Regsiter
> >> 2. mmc_register_driver --  MMC regsiter
> >>
> >> and filling the mmc_driver structure.
> >>
> >> I am not able to see the probe of MMC, But i see the return value of
> >> mmc_register function returning success.
> > I am not an expert on MMC driver but AFAIK it is no different in terms
> > of following device/driver model.
> > Probe of a function is only called when device name matches with driver
> > name and when it happens driver calls your probe.
> >
> > So in your case even though you have registered the driver, looks like
> > you are missing the device registration part.Do that and see the magic.
> > If this is SOC then that is done in the board file i.e.
> > arch/arm/plat-xyz/
> >
> >>
> >> Kindly let me know how i make the probe of mmc getting called
> >>
> >> Thanks
> >>
> >>
> >> Chetan
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> >> the body of a message to majord...@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >> Please read the FAQ at  http://www.tux.org/lkml/
> >
> >


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Fwd: mmc card probe not getting called

[anish kumar]

On Tue, 2013-02-19 at 12:16 +0530, chetan cr123 wrote:
> HI All,
> 
> I am working on Sd Card/Block driver
> 
> I am registering it as both
> 
> 1. register_blkdev()-  BLOCK Regsiter
> 2. mmc_register_driver --  MMC regsiter
> 
> and filling the mmc_driver structure.
> 
> I am not able to see the probe of MMC, But i see the return value of
> mmc_register function returning success.
I am not an expert on MMC driver but AFAIK it is no different in terms
of following device/driver model.
Probe of a function is only called when device name matches with driver
name and when it happens driver calls your probe.

So in your case even though you have registered the driver, looks like
you are missing the device registration part.Do that and see the magic.
If this is SOC then that is done in the board file i.e.
arch/arm/plat-xyz/

> 
> Kindly let me know how i make the probe of mmc getting called
> 
> Thanks
> 
> 
> Chetan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Fwd: mmc card probe not getting called

[anish kumar]

On Tue, 2013-02-19 at 12:16 +0530, chetan cr123 wrote:
 HI All,
 
 I am working on Sd Card/Block driver
 
 I am registering it as both
 
 1. register_blkdev()-  BLOCK Regsiter
 2. mmc_register_driver --  MMC regsiter
 
 and filling the mmc_driver structure.
 
 I am not able to see the probe of MMC, But i see the return value of
 mmc_register function returning success.
I am not an expert on MMC driver but AFAIK it is no different in terms
of following device/driver model.
Probe of a function is only called when device name matches with driver
name and when it happens driver calls your probe.

So in your case even though you have registered the driver, looks like
you are missing the device registration part.Do that and see the magic.
If this is SOC then that is done in the board file i.e.
arch/arm/plat-xyz/

 
 Kindly let me know how i make the probe of mmc getting called
 
 Thanks
 
 
 Chetan
 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Fwd: mmc card probe not getting called

[anish kumar]

On Wed, 2013-02-20 at 11:25 +0530, chetan cr123 wrote:
Avoiding top posting.
 Hi Anish,
 
 Thanks for your reply,
 
 I was doing device registration for device by giving same name as
 driver name, This i used to do in platform driver registration,
 
 But i dont know how to do for mmc device registration,
 
 And i also want to know which part of the code(file name) is doing the
 string compare with the driver and device names and calling the probe
 function. can u please point me to that part of code. from many days i
 was searching from which part of code where string compare is done and
 calls the probe function.
 
 
 Kindly point me out to that part of code.
look at drivers/base/dd.c
static int really_probe(struct device *dev, struct device_driver *drv)
{
//snip
if (dev-bus-probe) {
ret = dev-bus-probe(dev);
if (ret)
goto probe_failed;
} else if (drv-probe) {
ret = drv-probe(dev);
if (ret)
goto probe_failed;
}
Tip:Whenever you want to see how some function is being called use
dump_stack().This will give you the call chain leading up to your
function call which you are interested in.
 
 
 
 On Tue, Feb 19, 2013 at 9:25 PM, anish kumar
 anish198519851...@gmail.com wrote:
  On Tue, 2013-02-19 at 12:16 +0530, chetan cr123 wrote:
  HI All,
 
  I am working on Sd Card/Block driver
 
  I am registering it as both
 
  1. register_blkdev()-  BLOCK Regsiter
  2. mmc_register_driver --  MMC regsiter
 
  and filling the mmc_driver structure.
 
  I am not able to see the probe of MMC, But i see the return value of
  mmc_register function returning success.
  I am not an expert on MMC driver but AFAIK it is no different in terms
  of following device/driver model.
  Probe of a function is only called when device name matches with driver
  name and when it happens driver calls your probe.
 
  So in your case even though you have registered the driver, looks like
  you are missing the device registration part.Do that and see the magic.
  If this is SOC then that is done in the board file i.e.
  arch/arm/plat-xyz/
 
 
  Kindly let me know how i make the probe of mmc getting called
 
  Thanks
 
 
  Chetan
  --
  To unsubscribe from this list: send the line unsubscribe linux-kernel in
  the body of a message to majord...@vger.kernel.org
  More majordomo info at  http://vger.kernel.org/majordomo-info.html
  Please read the FAQ at  http://www.tux.org/lkml/
 
 


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Stupid user with user-space questions, matrix LED driving with user space code only.

[anish kumar]

On Sun, 2013-02-17 at 14:37 +, Jonathan Andrews wrote:
> > > The dim pixel code is timing critical (but as I said only a tiny
> > > fraction of the total CPU usage). What I need to do is grab the CPU and
> > > prevent any context switch (IRQ or PREEMPT) for this period.
> > why you want to do this?
> Because I need an I/O pin to change state for a short accurately timed
> period. If the scheduler is activated during the generation of this
> pulse then the timing will be stretched, this stretching is seen as
> bright flash by the viewer of the LED display. 
You can do that in kernel.
> 
> >  
> > > I cant find a user space mechanism other than changing the kernel to
> > > disable preemtion ? No simple /proc switch to turn it on/off ? If not
> > > why - I cant be the only one who wants to toggle preemption off without
> > > swapping kernels ?
> > you can't disable pre-emption from user space.
> Part of why I asked this here.
> 
> Why not !
> 
> I would expect a "/proc/sys/kernel/sched_preemption" that I could toggle
> a 1 or 0 into to turn it on/off.  
Do it in kernel and that is why drivers exist in the kernel.
> 
> >From a user perspective it seems a bit crap to have to change the kernel
> if you have a workload that preemption is harmful to.  
> In the case of something like the Raspberry Pi changing the kernel if
> the distribution has not done the work for me sounds like real effort.
> The kernel is tied to binary obscurity from broadcom... To build I need
> a working cross compiler, toolchain, kernel sources, Pi specific patches
> then to get everything in the correct place on an SD card containing two
> filesystems.  Its possible but its not going to "just work" at my skill
> level
AFAIK raspberry pi kernel is very easy to build and there are already
source code out there which you just need to clone and compile.This
includes all the necessary tools so...just try and fail before
deciding!!
> > > The other issue is that of IRQs, my dim pixels on the display seem to
> > > flash brighter from time to time, this I assume is partly preemption
> > > (maybe possibly) and partly IRQ handling (more likely) allowing context
> > > switches or just taking a while on slow hardware.
> > > 
> > > I need only a tiny fraction of the runtime to be hard real time, on
> > > intel in the past i've simply disabled IRQs briefly with some inline
> > > assembler. 
> > you shouldn't fiddle with irq's from user space but...
> > > The Raspberry Pi board would also probably survive this as the only
> > > active peripheral is ethenet, I suspect couple of missed IRQs would not
> > > matter as once IRQs are re-enabled the USB/ethernet hardware will likely
> > > have the data or it can be re-tried.  Does anyone have an example of a
> > > dirty hack along these lines they can share with me :-) 
> > 
> > > Do I have any simple mechanism available to disable (or defer) kernel
> > > IRQ handling briefly from user space code, I suspect not but no harm in
> > > asking ?
> > Use any sysfs to disable/enable the irq. This approach is very bad but
> > as you said you wanted a hack.
> Sounds interesting, can I have some more specific clues how. Device is
> not intel or PCI so I need to toggle something relevant to ARMs native
> interrupt system, do I still have anything I can toggle that is relevant
> in "Linux raspberrypi 3.6.11+ #375 PREEMPT" /sys ?
So try building the raspberry kernel.
> 
> Many thanks,
> Jon
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Stupid user with user-space questions, matrix LED driving with user space code only.

[anish kumar]

On Sun, 2013-02-17 at 14:37 +, Jonathan Andrews wrote:
   The dim pixel code is timing critical (but as I said only a tiny
   fraction of the total CPU usage). What I need to do is grab the CPU and
   prevent any context switch (IRQ or PREEMPT) for this period.
  why you want to do this?
 Because I need an I/O pin to change state for a short accurately timed
 period. If the scheduler is activated during the generation of this
 pulse then the timing will be stretched, this stretching is seen as
 bright flash by the viewer of the LED display. 
You can do that in kernel.
 
   
   I cant find a user space mechanism other than changing the kernel to
   disable preemtion ? No simple /proc switch to turn it on/off ? If not
   why - I cant be the only one who wants to toggle preemption off without
   swapping kernels ?
  you can't disable pre-emption from user space.
 Part of why I asked this here.
 
 Why not !
 
 I would expect a /proc/sys/kernel/sched_preemption that I could toggle
 a 1 or 0 into to turn it on/off.  
Do it in kernel and that is why drivers exist in the kernel.
 
 From a user perspective it seems a bit crap to have to change the kernel
 if you have a workload that preemption is harmful to.  
 In the case of something like the Raspberry Pi changing the kernel if
 the distribution has not done the work for me sounds like real effort.
 The kernel is tied to binary obscurity from broadcom... To build I need
 a working cross compiler, toolchain, kernel sources, Pi specific patches
 then to get everything in the correct place on an SD card containing two
 filesystems.  Its possible but its not going to just work at my skill
 level
AFAIK raspberry pi kernel is very easy to build and there are already
source code out there which you just need to clone and compile.This
includes all the necessary tools so...just try and fail before
deciding!!
   The other issue is that of IRQs, my dim pixels on the display seem to
   flash brighter from time to time, this I assume is partly preemption
   (maybe possibly) and partly IRQ handling (more likely) allowing context
   switches or just taking a while on slow hardware.
   
   I need only a tiny fraction of the runtime to be hard real time, on
   intel in the past i've simply disabled IRQs briefly with some inline
   assembler. 
  you shouldn't fiddle with irq's from user space but...
   The Raspberry Pi board would also probably survive this as the only
   active peripheral is ethenet, I suspect couple of missed IRQs would not
   matter as once IRQs are re-enabled the USB/ethernet hardware will likely
   have the data or it can be re-tried.  Does anyone have an example of a
   dirty hack along these lines they can share with me :-) 
  
   Do I have any simple mechanism available to disable (or defer) kernel
   IRQ handling briefly from user space code, I suspect not but no harm in
   asking ?
  Use any sysfs to disable/enable the irq. This approach is very bad but
  as you said you wanted a hack.
 Sounds interesting, can I have some more specific clues how. Device is
 not intel or PCI so I need to toggle something relevant to ARMs native
 interrupt system, do I still have anything I can toggle that is relevant
 in Linux raspberrypi 3.6.11+ #375 PREEMPT /sys ?
So try building the raspberry kernel.
 
 Many thanks,
 Jon
 
 


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Stupid user with user-space questions, matrix LED driving with user space code only.

[anish kumar]

On Sat, 2013-02-16 at 14:37 +, Jonathan Andrews wrote:
> I hope this is the correct place, I expect to get abused.
> 
> I'm trying to do a mostly soft real-time task with a very small hard
> real time element.
> 
> I've written some code to drive matrix LED signs using a Raspberry Pi.
> 
> Source here:
> http://www.jonshouse.co.uk/download/128x32_red_green_led_sign.tar.gz
> 
> 
> Since I last used linux you kernel people have fiddled with it yet
> again:
> 
> Linux raspberrypi 3.6.11+ #375 PREEMPT Tue Feb 12 01:41:07 GMT 2013 armv6l 
> GNU/Linux
> 
> I'm scanning an LED display to produce a 2 bits per pixel image.  The
> code simply alters the amount of time any one LED is on, for higher
> intensity pixels the true amount of "on" time is non critical.
> 
> I've marked my process as realtime.
> 
> My problem is that for very dim pixels the amount of "on" time for the
> LED  is very critical, this is only a fraction of a percent of the total
> processes timeslice.
> It scans 100 frames of brightest, 100 frames of brighter and 1 frame of
> dim pixels for example, so 200:1 ratio of don't care much /vs care a lot
> timing !
> 
> To this end I'm using a hard coded small delay instead of usleep for the
> tight timing section:
> 
> // Delay for ARM without yeilding to schedular, roughly calibrated but better 
> than usleep for short delays
> inline usleep_arm_hard(int usecs)
> {
> long int outer,inner;
> 
> for (outer=0;outer {
> for (inner=0;inner<300;inner++)
> asm("andeq r0, r0, r0");  // NOP
> }
> }
> 
> The dim pixel code is timing critical (but as I said only a tiny
> fraction of the total CPU usage). What I need to do is grab the CPU and
> prevent any context switch (IRQ or PREEMPT) for this period.
why you want to do this?
> 
> I cant find a user space mechanism other than changing the kernel to
> disable preemtion ? No simple /proc switch to turn it on/off ? If not
> why - I cant be the only one who wants to toggle preemption off without
> swapping kernels ?
you can't disable pre-emption from user space.
> 
> The other issue is that of IRQs, my dim pixels on the display seem to
> flash brighter from time to time, this I assume is partly preemption
> (maybe possibly) and partly IRQ handling (more likely) allowing context
> switches or just taking a while on slow hardware.
> 
> I need only a tiny fraction of the runtime to be hard real time, on
> intel in the past i've simply disabled IRQs briefly with some inline
> assembler. 
you shouldn't fiddle with irq's from user space but...
> The Raspberry Pi board would also probably survive this as the only
> active peripheral is ethenet, I suspect couple of missed IRQs would not
> matter as once IRQs are re-enabled the USB/ethernet hardware will likely
> have the data or it can be re-tried.  Does anyone have an example of a
> dirty hack along these lines they can share with me :-) 

> Do I have any simple mechanism available to disable (or defer) kernel
> IRQ handling briefly from user space code, I suspect not but no harm in
> asking ?
Use any sysfs to disable/enable the irq. This approach is very bad but
as you said you wanted a hack.
> Thanks,
> Jon
> 
> PS I'm not a kernel hacker - yes I know I could write a proper driver
> for the task but I lack any real skill and the required time !
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable

[anish kumar]

From: anish kumar 

This watchdog_disabled flag is bit of cryptic.Howerver it's usefullnes is 
multifold.
Uses are:
1. Check if smpboot_register_percpu_thread function passed.
2. Makes sure that user enables and disables the watchdog in sequence
   i.e. enable watchdog->disable watchdog->enable watchdog
   Unlike enable watchdog->enable watchdog which is wrong.

Signed-off-by: anish kumar 
---
 kernel/watchdog.c |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 75a2ab3..8a20ebe 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -518,6 +518,11 @@ int proc_dowatchdog(struct ctl_table *table, int write,
return ret;
 
set_sample_period();
+   /*
+* Watchdog threads shouldn't be enabled if they are
+* disabled.'watchdog_disabled' variable check in
+* watchdog_*_all_cpus() function takes care of this.
+*/
if (watchdog_enabled && watchdog_thresh)
watchdog_enable_all_cpus();
else
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable

[anish kumar]

On Sat, 2013-02-16 at 09:42 +0100, Ingo Molnar wrote:
> * Don Zickus  wrote:
> 
> > > >> > +   /*
> > > >> > +* We shouldn't enable watchdog threads if it is
> > > >> > +* disabled.This is done by watchdog_disabled
> > > >> > +* variable check in watchdog_*_all_cpus function.
> > > >
> > > > It has two grammatic and a stylistic error in it, plus misses
> > > Would you mind pointing it out to me the grammatical mistakes
> > > as I am not that good with grammar.
> > 
> > I am not entirely sure which ones Ingo is referring to, but what I see are
> > 
> > 'disabled.This' needs a space after period
> > 'This is done by watchdog_disabled' needs a 'the' after 'by'
> > 'variable check in watchdog..' needs a 'the' after 'in'.
> > 
> > in addition to the missing ()'s after 'watchdog_*_all_cpus.
> 
> There's also a plural/singular mismatch between 'watchdog 
> threads' and 'if it is disabled'.
Thanks, I will update the same in the next patch.
> 
> Thanks,
> 
>   Ingo


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable

[anish kumar]

From: anish kumar anish198519851...@gmail.com

This watchdog_disabled flag is bit of cryptic.Howerver it's usefullnes is 
multifold.
Uses are:
1. Check if smpboot_register_percpu_thread function passed.
2. Makes sure that user enables and disables the watchdog in sequence
   i.e. enable watchdog-disable watchdog-enable watchdog
   Unlike enable watchdog-enable watchdog which is wrong.

Signed-off-by: anish kumar anish198519851...@gmail.com
---
 kernel/watchdog.c |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 75a2ab3..8a20ebe 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -518,6 +518,11 @@ int proc_dowatchdog(struct ctl_table *table, int write,
return ret;
 
set_sample_period();
+   /*
+* Watchdog threads shouldn't be enabled if they are
+* disabled.'watchdog_disabled' variable check in
+* watchdog_*_all_cpus() function takes care of this.
+*/
if (watchdog_enabled  watchdog_thresh)
watchdog_enable_all_cpus();
else
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Stupid user with user-space questions, matrix LED driving with user space code only.

[anish kumar]

On Sat, 2013-02-16 at 14:37 +, Jonathan Andrews wrote:
 I hope this is the correct place, I expect to get abused.
 
 I'm trying to do a mostly soft real-time task with a very small hard
 real time element.
 
 I've written some code to drive matrix LED signs using a Raspberry Pi.
 
 Source here:
 http://www.jonshouse.co.uk/download/128x32_red_green_led_sign.tar.gz
 
 
 Since I last used linux you kernel people have fiddled with it yet
 again:
 
 Linux raspberrypi 3.6.11+ #375 PREEMPT Tue Feb 12 01:41:07 GMT 2013 armv6l 
 GNU/Linux
 
 I'm scanning an LED display to produce a 2 bits per pixel image.  The
 code simply alters the amount of time any one LED is on, for higher
 intensity pixels the true amount of on time is non critical.
 
 I've marked my process as realtime.
 
 My problem is that for very dim pixels the amount of on time for the
 LED  is very critical, this is only a fraction of a percent of the total
 processes timeslice.
 It scans 100 frames of brightest, 100 frames of brighter and 1 frame of
 dim pixels for example, so 200:1 ratio of don't care much /vs care a lot
 timing !
 
 To this end I'm using a hard coded small delay instead of usleep for the
 tight timing section:
 
 // Delay for ARM without yeilding to schedular, roughly calibrated but better 
 than usleep for short delays
 inline usleep_arm_hard(int usecs)
 {
 long int outer,inner;
 
 for (outer=0;outerusecs;outer++)
 {
 for (inner=0;inner300;inner++)
 asm(andeq r0, r0, r0);  // NOP
 }
 }
 
 The dim pixel code is timing critical (but as I said only a tiny
 fraction of the total CPU usage). What I need to do is grab the CPU and
 prevent any context switch (IRQ or PREEMPT) for this period.
why you want to do this?
 
 I cant find a user space mechanism other than changing the kernel to
 disable preemtion ? No simple /proc switch to turn it on/off ? If not
 why - I cant be the only one who wants to toggle preemption off without
 swapping kernels ?
you can't disable pre-emption from user space.
 
 The other issue is that of IRQs, my dim pixels on the display seem to
 flash brighter from time to time, this I assume is partly preemption
 (maybe possibly) and partly IRQ handling (more likely) allowing context
 switches or just taking a while on slow hardware.
 
 I need only a tiny fraction of the runtime to be hard real time, on
 intel in the past i've simply disabled IRQs briefly with some inline
 assembler. 
you shouldn't fiddle with irq's from user space but...
 The Raspberry Pi board would also probably survive this as the only
 active peripheral is ethenet, I suspect couple of missed IRQs would not
 matter as once IRQs are re-enabled the USB/ethernet hardware will likely
 have the data or it can be re-tried.  Does anyone have an example of a
 dirty hack along these lines they can share with me :-) 

 Do I have any simple mechanism available to disable (or defer) kernel
 IRQ handling briefly from user space code, I suspect not but no harm in
 asking ?
Use any sysfs to disable/enable the irq. This approach is very bad but
as you said you wanted a hack.
 Thanks,
 Jon
 
 PS I'm not a kernel hacker - yes I know I could write a proper driver
 for the task but I lack any real skill and the required time !
 
 
 
 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] driver core: add wait event for deferred probe

[anish kumar]

On Thu, 2013-02-14 at 16:33 +, Grant Likely wrote:
> On Thu, 14 Feb 2013 08:57:17 +0530, anish singh  
> wrote:
> > On Thu, Feb 14, 2013 at 3:06 AM, Grant Likely  
> > wrote:
> > >  static int deferred_probe_initcall(void)
> > >  {
> > > deferred_wq = create_singlethread_workqueue("deferwq");
> > > if (WARN_ON(!deferred_wq))
> > > return -ENOMEM;
> > >
> > > driver_deferred_probe_enable = true;
> > > +   deferred_probe_work_func(NULL);
> > > -   driver_deferred_probe_trigger();
> > > return 0;
> > >  }
> > >  late_initcall(deferred_probe_initcall);
> > >
> > > Or something similar. That would guarantee that as many passes as are 
> > > needed
> > > (which in practical terms only means a couple) for device probing to
> > > settle down before exiting the initcall processing. That should achieve
> > > the effect you desire.
> > >
> > > It still masks the __init section issue by making it a lot less likely,
> > Grant, Can you please explain me this problem?My understanding is below:
> > If all the detection of devices with there respective driver is done before
> > __init section is freed then we will not have the problem mentioned.
> > However if the driver requests the probing to be deferred then __init 
> > section
> > of the deferred driver will not be freed right?
> > 
> > I am afraid but the patch description is bit cryptic for me specially
> > this line "kernel has to open console failure & release __init section 
> > before
> > reinvoking failure".
> 
> Yes I can, but first I'll briefly describe the Linux driver model to make sure
> we're talking about the same thing...
> 
> drivercore in Linux is oriented around two data structures:
> 1) Devices (struct device), and
> 2) Drivers (struct device_driver)
> 
> Hardware is modeled with instances of 'struct device'. For each device
> that Linux knows about there is one 'struct device'[1]. The devices are
> organized into a hierarchical tree, and you can see it by looking in
> /sys/devices.
> 
> Device drivers are represended by struct device_driver. Each driver,
> whether built-in or a module, is responsible to register itself by
> embedding a struct device_driver, or a structure that contains a struct
> device_driver. For example, struct platform_driver has an embedded
> struct device_driver.
> 
> The whole purpose of drivercore is to match up drivers to devices. Each
> bus_type has its own mechanism for deciding which devices and
> device_drivers go together, but it still results in trying to bind a
> struct device_driver to each struct device.
> 
> An important detail here is that drivercore is entirely asynchronous.
> There are no requirements on what order devices and device_drivers are
> registered. When a device gets registered, drivercore attempts to bind
> it to any device_driver that it already knows about. Similarly, when a
> new device_driver gets registered, drivercore will see if there are any
> unbound devices that it can bind it to. It is even possible to trigger a
> bind attempt sometime after both device and device_driver have been
> registered.[2]
> 
> This is the reason that deferred_probe is an option. As long as the
> kernel keeps track of which device_drivers requested deferred probe, we
> can nudge drivercore to reattempt probing. It really doesn't matter what
> order or when drivers get bound...
> 
> ...except when it does. Here's where we get into the issues related to
> __init sections and deferred probe. Since the driver model can bind a
> driver at any time, including after userspace has started, the
> expectation is that none of the code paths associated with probing will
> be discarded. That is why .probe hooks cannot be __init annotated. The
> problem for consoles is that the console init hook gets called from
> the probe path and a lot of console init hooks are __init annotated.
> 
> Before deferred probe, this was rarely (if ever) an actual problem. In
> general the order of operations during kernel init is:
> 
> 1) (early boot) create and register a bunch of struct devices
> 2) (initcalls) register a bunch of struct device_drivers
>  - a bunch of binds happen at this point as device drivers get
>registered
>  - This is why device initialization order is primarily driven by driver
>link order.
> 3) discard __init sections
> 4) userspace.
> 
> It's not that bind is guaranteed to occur before __init discard, but
> rather nothing prevented it from happening after. Deferred probe changes
> that. With deferred probe, Haojian correctly analyzed that driver bind
> is getting pushed after __init is discarded.
> 
> However, the problem with his solution was that it assumed that *all*
> deferred drivers must be resolved before proceeding with init which
> cannot be guaranteed. It is absolutely possble for a built-in driver to
> depend on something provided by a module. As rmk pointed out, that is
> not okay for console devices, so it is still important to make sure

Re: [PATCH] driver core: add wait event for deferred probe

[anish kumar]

On Thu, 2013-02-14 at 16:33 +, Grant Likely wrote:
 On Thu, 14 Feb 2013 08:57:17 +0530, anish singh anish198519851...@gmail.com 
 wrote:
  On Thu, Feb 14, 2013 at 3:06 AM, Grant Likely grant.lik...@secretlab.ca 
  wrote:
static int deferred_probe_initcall(void)
{
   deferred_wq = create_singlethread_workqueue(deferwq);
   if (WARN_ON(!deferred_wq))
   return -ENOMEM;
  
   driver_deferred_probe_enable = true;
   +   deferred_probe_work_func(NULL);
   -   driver_deferred_probe_trigger();
   return 0;
}
late_initcall(deferred_probe_initcall);
  
   Or something similar. That would guarantee that as many passes as are 
   needed
   (which in practical terms only means a couple) for device probing to
   settle down before exiting the initcall processing. That should achieve
   the effect you desire.
  
   It still masks the __init section issue by making it a lot less likely,
  Grant, Can you please explain me this problem?My understanding is below:
  If all the detection of devices with there respective driver is done before
  __init section is freed then we will not have the problem mentioned.
  However if the driver requests the probing to be deferred then __init 
  section
  of the deferred driver will not be freed right?
  
  I am afraid but the patch description is bit cryptic for me specially
  this line kernel has to open console failure  release __init section 
  before
  reinvoking failure.
 
 Yes I can, but first I'll briefly describe the Linux driver model to make sure
 we're talking about the same thing...
 
 drivercore in Linux is oriented around two data structures:
 1) Devices (struct device), and
 2) Drivers (struct device_driver)
 
 Hardware is modeled with instances of 'struct device'. For each device
 that Linux knows about there is one 'struct device'[1]. The devices are
 organized into a hierarchical tree, and you can see it by looking in
 /sys/devices.
 
 Device drivers are represended by struct device_driver. Each driver,
 whether built-in or a module, is responsible to register itself by
 embedding a struct device_driver, or a structure that contains a struct
 device_driver. For example, struct platform_driver has an embedded
 struct device_driver.
 
 The whole purpose of drivercore is to match up drivers to devices. Each
 bus_type has its own mechanism for deciding which devices and
 device_drivers go together, but it still results in trying to bind a
 struct device_driver to each struct device.
 
 An important detail here is that drivercore is entirely asynchronous.
 There are no requirements on what order devices and device_drivers are
 registered. When a device gets registered, drivercore attempts to bind
 it to any device_driver that it already knows about. Similarly, when a
 new device_driver gets registered, drivercore will see if there are any
 unbound devices that it can bind it to. It is even possible to trigger a
 bind attempt sometime after both device and device_driver have been
 registered.[2]
 
 This is the reason that deferred_probe is an option. As long as the
 kernel keeps track of which device_drivers requested deferred probe, we
 can nudge drivercore to reattempt probing. It really doesn't matter what
 order or when drivers get bound...
 
 ...except when it does. Here's where we get into the issues related to
 __init sections and deferred probe. Since the driver model can bind a
 driver at any time, including after userspace has started, the
 expectation is that none of the code paths associated with probing will
 be discarded. That is why .probe hooks cannot be __init annotated. The
 problem for consoles is that the console init hook gets called from
 the probe path and a lot of console init hooks are __init annotated.
 
 Before deferred probe, this was rarely (if ever) an actual problem. In
 general the order of operations during kernel init is:
 
 1) (early boot) create and register a bunch of struct devices
 2) (initcalls) register a bunch of struct device_drivers
  - a bunch of binds happen at this point as device drivers get
registered
  - This is why device initialization order is primarily driven by driver
link order.
 3) discard __init sections
 4) userspace.
 
 It's not that bind is guaranteed to occur before __init discard, but
 rather nothing prevented it from happening after. Deferred probe changes
 that. With deferred probe, Haojian correctly analyzed that driver bind
 is getting pushed after __init is discarded.
 
 However, the problem with his solution was that it assumed that *all*
 deferred drivers must be resolved before proceeding with init which
 cannot be guaranteed. It is absolutely possble for a built-in driver to
 depend on something provided by a module. As rmk pointed out, that is
 not okay for console devices, so it is still important to make sure
 everything needed for the console is built-in, but non-critical devices
 may not care.
 
 The difference 

Re: [PATCH v2] driver core: add wait event for deferred probe

[anish kumar]

On Tue, 2013-02-12 at 10:58 +0800, Haojian Zhuang wrote:
> do_initcalls() could call all driver initialization code in kernel_init
> thread. It means that probe() function will be also called from that
> time. After this, kernel could access console & release __init section
> in the same thread.
> 
> But if device probe fails and moves into deferred probe list, a new
> thread is created to reinvoke probe. If the device is serial console,
> kernel has to open console failure & release __init section before
> reinvoking failure. Because there's no synchronization mechanism.
> Now add wait event to synchronize after do_initcalls().
Can you describe the problem you are facing in detail i.e. without this
patch what is happening weird?
> 
> Changelog:
> v2: add comments on wait_for_device_probe().
> 
> Signed-off-by: Haojian Zhuang 
> ---
>  drivers/base/dd.c |8 +---
>  init/main.c   |2 ++
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 65631015..23db672 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -53,6 +53,9 @@ static LIST_HEAD(deferred_probe_pending_list);
>  static LIST_HEAD(deferred_probe_active_list);
>  static struct workqueue_struct *deferred_wq;
>  
> +static atomic_t probe_count = ATOMIC_INIT(0);
> +static DECLARE_WAIT_QUEUE_HEAD(probe_waitqueue);
> +
>  /**
>   * deferred_probe_work_func() - Retry probing devices in the active list.
>   */
> @@ -77,6 +80,7 @@ static void deferred_probe_work_func(struct work_struct 
> *work)
>   private = list_first_entry(_probe_active_list,
>   typeof(*dev->p), deferred_probe);
>   dev = private->device;
> + atomic_dec(_count);
>   list_del_init(>deferred_probe);
>  
>   get_device(dev);
> @@ -257,9 +261,6 @@ int device_bind_driver(struct device *dev)
>  }
>  EXPORT_SYMBOL_GPL(device_bind_driver);
>  
> -static atomic_t probe_count = ATOMIC_INIT(0);
> -static DECLARE_WAIT_QUEUE_HEAD(probe_waitqueue);
> -
>  static int really_probe(struct device *dev, struct device_driver *drv)
>  {
>   int ret = 0;
> @@ -308,6 +309,7 @@ probe_failed:
>   /* Driver requested deferred probing */
>   dev_info(dev, "Driver %s requests probe deferral\n", drv->name);
>   driver_deferred_probe_add(dev);
> + atomic_inc(_count);
>   } else if (ret != -ENODEV && ret != -ENXIO) {
>   /* driver matched but the probe failed */
>   printk(KERN_WARNING
> diff --git a/init/main.c b/init/main.c
> index 63534a1..141a6a4 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -786,6 +786,8 @@ static void __init do_basic_setup(void)
>   do_ctors();
>   usermodehelper_enable();
>   do_initcalls();
> + /* wait all deferred probe finished & prepare to drop __init section */
> + wait_for_device_probe();

static int __ref kernel_init(void *unused)
{
kernel_init_freeable();
/* need to finish all async __init code before freeing the
memory */
async_synchronize_full();

Is not your wait_for_device_probe() also calling
async_synchronize_full() function?AFAIK this function will send all the
instructions before moving ahead no?My understanding originates from the
below text.

>From kernel/async.c file
Subsystem/driver initialization code that scheduled asynchronous probe
functions, but which shares global resources with other
drivers/subsystems that do not use the asynchronous call feature, need
to do a full synchronization with the async_synchronize_full() function,
before returning from their init function. This is to maintain strict
ordering between the asynchronous and synchronous parts of the kernel.

>  }
>  
>  static void __init do_pre_smp_initcalls(void)


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] driver core: add wait event for deferred probe

[anish kumar]

On Tue, 2013-02-12 at 10:58 +0800, Haojian Zhuang wrote:
 do_initcalls() could call all driver initialization code in kernel_init
 thread. It means that probe() function will be also called from that
 time. After this, kernel could access console  release __init section
 in the same thread.
 
 But if device probe fails and moves into deferred probe list, a new
 thread is created to reinvoke probe. If the device is serial console,
 kernel has to open console failure  release __init section before
 reinvoking failure. Because there's no synchronization mechanism.
 Now add wait event to synchronize after do_initcalls().
Can you describe the problem you are facing in detail i.e. without this
patch what is happening weird?
 
 Changelog:
 v2: add comments on wait_for_device_probe().
 
 Signed-off-by: Haojian Zhuang haojian.zhu...@linaro.org
 ---
  drivers/base/dd.c |8 +---
  init/main.c   |2 ++
  2 files changed, 7 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/base/dd.c b/drivers/base/dd.c
 index 65631015..23db672 100644
 --- a/drivers/base/dd.c
 +++ b/drivers/base/dd.c
 @@ -53,6 +53,9 @@ static LIST_HEAD(deferred_probe_pending_list);
  static LIST_HEAD(deferred_probe_active_list);
  static struct workqueue_struct *deferred_wq;
  
 +static atomic_t probe_count = ATOMIC_INIT(0);
 +static DECLARE_WAIT_QUEUE_HEAD(probe_waitqueue);
 +
  /**
   * deferred_probe_work_func() - Retry probing devices in the active list.
   */
 @@ -77,6 +80,7 @@ static void deferred_probe_work_func(struct work_struct 
 *work)
   private = list_first_entry(deferred_probe_active_list,
   typeof(*dev-p), deferred_probe);
   dev = private-device;
 + atomic_dec(probe_count);
   list_del_init(private-deferred_probe);
  
   get_device(dev);
 @@ -257,9 +261,6 @@ int device_bind_driver(struct device *dev)
  }
  EXPORT_SYMBOL_GPL(device_bind_driver);
  
 -static atomic_t probe_count = ATOMIC_INIT(0);
 -static DECLARE_WAIT_QUEUE_HEAD(probe_waitqueue);
 -
  static int really_probe(struct device *dev, struct device_driver *drv)
  {
   int ret = 0;
 @@ -308,6 +309,7 @@ probe_failed:
   /* Driver requested deferred probing */
   dev_info(dev, Driver %s requests probe deferral\n, drv-name);
   driver_deferred_probe_add(dev);
 + atomic_inc(probe_count);
   } else if (ret != -ENODEV  ret != -ENXIO) {
   /* driver matched but the probe failed */
   printk(KERN_WARNING
 diff --git a/init/main.c b/init/main.c
 index 63534a1..141a6a4 100644
 --- a/init/main.c
 +++ b/init/main.c
 @@ -786,6 +786,8 @@ static void __init do_basic_setup(void)
   do_ctors();
   usermodehelper_enable();
   do_initcalls();
 + /* wait all deferred probe finished  prepare to drop __init section */
 + wait_for_device_probe();

static int __ref kernel_init(void *unused)
{
kernel_init_freeable();
/* need to finish all async __init code before freeing the
memory */
async_synchronize_full();

Is not your wait_for_device_probe() also calling
async_synchronize_full() function?AFAIK this function will send all the
instructions before moving ahead no?My understanding originates from the
below text.

From kernel/async.c file
Subsystem/driver initialization code that scheduled asynchronous probe
functions, but which shares global resources with other
drivers/subsystems that do not use the asynchronous call feature, need
to do a full synchronization with the async_synchronize_full() function,
before returning from their init function. This is to maintain strict
ordering between the asynchronous and synchronous parts of the kernel.

  }
  
  static void __init do_pre_smp_initcalls(void)


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Kernel code interrupted by Timer

[anish kumar]

On Sat, 2013-02-09 at 14:57 +0800, Peter Teoh wrote:
> 
> 
> On Sat, Feb 9, 2013 at 1:47 PM, anish kumar 
> .
> Timer interrupts is supposed to cause scheduling and scheduler
> may or
> may not pick up your last process(we always use the term
> "task" in
> kernel space) after handling timer interrupt.
> >
> 
> 
> 
> Sorry if I may disagree, correct me if wrong.   Timer interrupt and
> scheduler is two different thing.   I just counted in the "drivers"
> subdirectory, there are at least more than 200 places where
> "setup_timer()" is called, and these have nothing to do with
> scheduling.   For eg, heartbeat operation etc.  Not sure I
> misunderstood something?
Have a look at kernel/timer.c and kernel/hrtimer.c.
There are many sched() calls in these files.This will invoke scheduler.
> 
> 
> -- 
> Regards,
> Peter Teoh


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Kernel code interrupted by Timer

[anish kumar]

On Sat, 2013-02-09 at 14:57 +0800, Peter Teoh wrote:
 
 
 On Sat, Feb 9, 2013 at 1:47 PM, anish kumar 
 .
 Timer interrupts is supposed to cause scheduling and scheduler
 may or
 may not pick up your last process(we always use the term
 task in
 kernel space) after handling timer interrupt.
 
 
 
 
 Sorry if I may disagree, correct me if wrong.   Timer interrupt and
 scheduler is two different thing.   I just counted in the drivers
 subdirectory, there are at least more than 200 places where
 setup_timer() is called, and these have nothing to do with
 scheduling.   For eg, heartbeat operation etc.  Not sure I
 misunderstood something?
Have a look at kernel/timer.c and kernel/hrtimer.c.
There are many sched() calls in these files.This will invoke scheduler.
 
 
 -- 
 Regards,
 Peter Teoh


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: pr_info not printing message in /var/log/messages

[anish kumar]

On Tue, 2013-02-05 at 16:18 -0500, valdis.kletni...@vt.edu wrote:
> On Wed, 06 Feb 2013 04:43:20 +0800, Jimmy Pan said:
> 
> > in fact, i've been always wondering what is the relationship between dmesg
> > and /var/log/message. they diverse a lot...
dmesg is provided by kernel using cat /proc/kmsg.

/proc/kmsg is like any other linux file which supports below file
operations:

from /kernel/printk.c
const struct file_operations kmsg_fops = {
.open = devkmsg_open,
.read = devkmsg_read,
.aio_write = devkmsg_writev,
.llseek = devkmsg_llseek,
.poll = devkmsg_poll,
.release = devkmsg_release,
};

printk dumps it's output in the ring buffer whose size is set using
defconfig CONFIG_LOG_BUF_SHIFT(if 16 => then ring buffer size is 64KB,
and for 17 => 128KB).

This ring buffer is the source for syslog and klogd daemon logs.How it
extracts the buffer depends on the configuration of these deamons.

Call stack:

printk
vprintk_emit
log_store
write to log_buf
log_from_idx used by /proc/kmsg to read the buffer
sylog uses ioctl to work on ring buffers:
case SYSLOG_ACTION_CLOSE:   /* Close log */
case SYSLOG_ACTION_OPEN:/* Open log */
case SYSLOG_ACTION_READ:/* Read from log */
case SYSLOG_ACTION_READ_CLEAR:
case SYSLOG_ACTION_READ_ALL:
case SYSLOG_ACTION_CLEAR:
case SYSLOG_ACTION_CONSOLE_OFF:
case SYSLOG_ACTION_CONSOLE_ON:
case SYSLOG_ACTION_CONSOLE_LEVEL:
case SYSLOG_ACTION_SIZE_UNREAD:
case SYSLOG_ACTION_SIZE_BUFFER:


Quoting from
http://askubuntu.com/questions/26237/difference-between-var-log-messages-var-log-syslog-and-var-log-kern-log

Syslog is a standard logging facility. It collects messages of various
programs and services including the kernel, and stores them, depending
on setup, in a bunch of log files typically under /var/log. There are
also possibilities to send the messages to another host over network, to
a serial console, to a database table, etc.

According to my /etc/syslog.conf, default /var/log/kern.log captures
only the kernel's messages of any loglevel; i.e. the output of dmesg.

/var/log/messages instead aims at storing valuable, non-debug and
non-critical messages. This log should be considered the "general system
activity" log.

/var/log/syslog in turn logs everything, except auth related messages.

Other insteresting standard logs managed by syslog
are /var/log/auth.log, /var/log/mail.log.

Regarding your question: if you need solely kernel messages log, use the
kern.log or call dmesg.
> 
> What ends up in /var/log/message is some subset (possibly 100%, possibly 0%)
> of what's in dmesg.  Where your syslog daemon routes stuff is a local config
> issue - if your syslogd supports it, there's no reason not to dump the 
> iptables
> messages in to /var/log/firewall and the rest of it in /var/log/kernel, or
> any other policy that makes sense for the sysadmin
> ___
> Kernelnewbies mailing list
> kernelnewb...@kernelnewbies.org
> http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: pr_info not printing message in /var/log/messages

[anish kumar]

On Tue, 2013-02-05 at 16:18 -0500, valdis.kletni...@vt.edu wrote:
 On Wed, 06 Feb 2013 04:43:20 +0800, Jimmy Pan said:
 
  in fact, i've been always wondering what is the relationship between dmesg
  and /var/log/message. they diverse a lot...
dmesg is provided by kernel using cat /proc/kmsg.

/proc/kmsg is like any other linux file which supports below file
operations:

from /kernel/printk.c
const struct file_operations kmsg_fops = {
.open = devkmsg_open,
.read = devkmsg_read,
.aio_write = devkmsg_writev,
.llseek = devkmsg_llseek,
.poll = devkmsg_poll,
.release = devkmsg_release,
};

printk dumps it's output in the ring buffer whose size is set using
defconfig CONFIG_LOG_BUF_SHIFT(if 16 = then ring buffer size is 64KB,
and for 17 = 128KB).

This ring buffer is the source for syslog and klogd daemon logs.How it
extracts the buffer depends on the configuration of these deamons.

Call stack:

printk
vprintk_emit
log_store
write to log_buf
log_from_idx used by /proc/kmsg to read the buffer
sylog uses ioctl to work on ring buffers:
case SYSLOG_ACTION_CLOSE:   /* Close log */
case SYSLOG_ACTION_OPEN:/* Open log */
case SYSLOG_ACTION_READ:/* Read from log */
case SYSLOG_ACTION_READ_CLEAR:
case SYSLOG_ACTION_READ_ALL:
case SYSLOG_ACTION_CLEAR:
case SYSLOG_ACTION_CONSOLE_OFF:
case SYSLOG_ACTION_CONSOLE_ON:
case SYSLOG_ACTION_CONSOLE_LEVEL:
case SYSLOG_ACTION_SIZE_UNREAD:
case SYSLOG_ACTION_SIZE_BUFFER:


Quoting from
http://askubuntu.com/questions/26237/difference-between-var-log-messages-var-log-syslog-and-var-log-kern-log

Syslog is a standard logging facility. It collects messages of various
programs and services including the kernel, and stores them, depending
on setup, in a bunch of log files typically under /var/log. There are
also possibilities to send the messages to another host over network, to
a serial console, to a database table, etc.

According to my /etc/syslog.conf, default /var/log/kern.log captures
only the kernel's messages of any loglevel; i.e. the output of dmesg.

/var/log/messages instead aims at storing valuable, non-debug and
non-critical messages. This log should be considered the general system
activity log.

/var/log/syslog in turn logs everything, except auth related messages.

Other insteresting standard logs managed by syslog
are /var/log/auth.log, /var/log/mail.log.

Regarding your question: if you need solely kernel messages log, use the
kern.log or call dmesg.
 
 What ends up in /var/log/message is some subset (possibly 100%, possibly 0%)
 of what's in dmesg.  Where your syslog daemon routes stuff is a local config
 issue - if your syslogd supports it, there's no reason not to dump the 
 iptables
 messages in to /var/log/firewall and the rest of it in /var/log/kernel, or
 any other policy that makes sense for the sysadmin
 ___
 Kernelnewbies mailing list
 kernelnewb...@kernelnewbies.org
 http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[tip:irq/core] irq_work: Remove return value from the irq_work_queue() function

[tip-bot for anish kumar]

Commit-ID:  c02cf5f8ed6137e2b3b2f10e0fca336e06e09ba4
Gitweb: http://git.kernel.org/tip/c02cf5f8ed6137e2b3b2f10e0fca336e06e09ba4
Author: anish kumar 
AuthorDate: Sun, 3 Feb 2013 22:08:23 +0100
Committer:  Ingo Molnar 
CommitDate: Mon, 4 Feb 2013 11:50:59 +0100

irq_work: Remove return value from the irq_work_queue() function

As no one is using the return value of irq_work_queue(),
so it is better to just make it void.

Signed-off-by: anish kumar 
Acked-by: Steven Rostedt 
[ Fix stale comments, remove now unnecessary __irq_work_queue() intermediate 
function ]
Signed-off-by: Frederic Weisbecker 
Link: 
http://lkml.kernel.org/r/1359925703-24304-1-git-send-email-fweis...@gmail.com
Signed-off-by: Ingo Molnar 
---
 include/linux/irq_work.h |  2 +-
 kernel/irq_work.c| 31 ++-
 2 files changed, 11 insertions(+), 22 deletions(-)

diff --git a/include/linux/irq_work.h b/include/linux/irq_work.h
index 6a9e8f5..ce60c08 100644
--- a/include/linux/irq_work.h
+++ b/include/linux/irq_work.h
@@ -16,7 +16,7 @@ void init_irq_work(struct irq_work *work, void (*func)(struct 
irq_work *))
work->func = func;
 }
 
-bool irq_work_queue(struct irq_work *work);
+void irq_work_queue(struct irq_work *work);
 void irq_work_run(void);
 void irq_work_sync(struct irq_work *work);
 
diff --git a/kernel/irq_work.c b/kernel/irq_work.c
index 64eddd5..c9d7478 100644
--- a/kernel/irq_work.c
+++ b/kernel/irq_work.c
@@ -63,12 +63,20 @@ void __weak arch_irq_work_raise(void)
 }
 
 /*
- * Queue the entry and raise the IPI if needed.
+ * Enqueue the irq_work @entry unless it's already pending
+ * somewhere.
+ *
+ * Can be re-enqueued while the callback is still in progress.
  */
-static void __irq_work_queue(struct irq_work *work)
+void irq_work_queue(struct irq_work *work)
 {
bool empty;
 
+   /* Only queue if not already pending */
+   if (!irq_work_claim(work))
+   return;
+
+   /* Queue the entry and raise the IPI if needed. */
preempt_disable();
 
empty = llist_add(>llnode, &__get_cpu_var(irq_work_list));
@@ -78,25 +86,6 @@ static void __irq_work_queue(struct irq_work *work)
 
preempt_enable();
 }
-
-/*
- * Enqueue the irq_work @entry, returns true on success, failure when the
- * @entry was already enqueued by someone else.
- *
- * Can be re-enqueued while the callback is still in progress.
- */
-bool irq_work_queue(struct irq_work *work)
-{
-   if (!irq_work_claim(work)) {
-   /*
-* Already enqueued, can't do!
-*/
-   return false;
-   }
-
-   __irq_work_queue(work);
-   return true;
-}
 EXPORT_SYMBOL_GPL(irq_work_queue);
 
 /*
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[tip:irq/core] irq_work: Remove return value from the irq_work_queue() function

[tip-bot for anish kumar]

Commit-ID:  c02cf5f8ed6137e2b3b2f10e0fca336e06e09ba4
Gitweb: http://git.kernel.org/tip/c02cf5f8ed6137e2b3b2f10e0fca336e06e09ba4
Author: anish kumar anish198519851...@gmail.com
AuthorDate: Sun, 3 Feb 2013 22:08:23 +0100
Committer:  Ingo Molnar mi...@kernel.org
CommitDate: Mon, 4 Feb 2013 11:50:59 +0100

irq_work: Remove return value from the irq_work_queue() function

As no one is using the return value of irq_work_queue(),
so it is better to just make it void.

Signed-off-by: anish kumar anish198519851...@gmail.com
Acked-by: Steven Rostedt rost...@goodmis.org
[ Fix stale comments, remove now unnecessary __irq_work_queue() intermediate 
function ]
Signed-off-by: Frederic Weisbecker fweis...@gmail.com
Link: 
http://lkml.kernel.org/r/1359925703-24304-1-git-send-email-fweis...@gmail.com
Signed-off-by: Ingo Molnar mi...@kernel.org
---
 include/linux/irq_work.h |  2 +-
 kernel/irq_work.c| 31 ++-
 2 files changed, 11 insertions(+), 22 deletions(-)

diff --git a/include/linux/irq_work.h b/include/linux/irq_work.h
index 6a9e8f5..ce60c08 100644
--- a/include/linux/irq_work.h
+++ b/include/linux/irq_work.h
@@ -16,7 +16,7 @@ void init_irq_work(struct irq_work *work, void (*func)(struct 
irq_work *))
work-func = func;
 }
 
-bool irq_work_queue(struct irq_work *work);
+void irq_work_queue(struct irq_work *work);
 void irq_work_run(void);
 void irq_work_sync(struct irq_work *work);
 
diff --git a/kernel/irq_work.c b/kernel/irq_work.c
index 64eddd5..c9d7478 100644
--- a/kernel/irq_work.c
+++ b/kernel/irq_work.c
@@ -63,12 +63,20 @@ void __weak arch_irq_work_raise(void)
 }
 
 /*
- * Queue the entry and raise the IPI if needed.
+ * Enqueue the irq_work @entry unless it's already pending
+ * somewhere.
+ *
+ * Can be re-enqueued while the callback is still in progress.
  */
-static void __irq_work_queue(struct irq_work *work)
+void irq_work_queue(struct irq_work *work)
 {
bool empty;
 
+   /* Only queue if not already pending */
+   if (!irq_work_claim(work))
+   return;
+
+   /* Queue the entry and raise the IPI if needed. */
preempt_disable();
 
empty = llist_add(work-llnode, __get_cpu_var(irq_work_list));
@@ -78,25 +86,6 @@ static void __irq_work_queue(struct irq_work *work)
 
preempt_enable();
 }
-
-/*
- * Enqueue the irq_work @entry, returns true on success, failure when the
- * @entry was already enqueued by someone else.
- *
- * Can be re-enqueued while the callback is still in progress.
- */
-bool irq_work_queue(struct irq_work *work)
-{
-   if (!irq_work_claim(work)) {
-   /*
-* Already enqueued, can't do!
-*/
-   return false;
-   }
-
-   __irq_work_queue(work);
-   return true;
-}
 EXPORT_SYMBOL_GPL(irq_work_queue);
 
 /*
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable

[anish kumar]

From: anish kumar 

This watchdog_disabled flag is bit of cryptic.Howerver it's usefullnes is 
multifold.
Uses are:
1. Check if smpboot_register_percpu_thread function passed.
2. Makes sure that user enables and disables the watchdog in sequence
   i.e. enable watchdog->disable watchdog->enable watchdog
   Unlike enable watchdog->enable watchdog which is wrong.

Signed-off-by: anish kumar 
---
 kernel/watchdog.c |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 75a2ab3..87a19aa 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -518,6 +518,11 @@ int proc_dowatchdog(struct ctl_table *table, int write,
return ret;
 
set_sample_period();
+   /*
+* We shouldn't enable watchdog threads if it is
+* disabled.This is done by watchdog_disabled
+* variable check in watchdog_*_all_cpus function.
+*/
if (watchdog_enabled && watchdog_thresh)
watchdog_enable_all_cpus();
else
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable

[anish kumar]

From: anish kumar anish198519851...@gmail.com

This watchdog_disabled flag is bit of cryptic.Howerver it's usefullnes is 
multifold.
Uses are:
1. Check if smpboot_register_percpu_thread function passed.
2. Makes sure that user enables and disables the watchdog in sequence
   i.e. enable watchdog-disable watchdog-enable watchdog
   Unlike enable watchdog-enable watchdog which is wrong.

Signed-off-by: anish kumar anish198519851...@gmail.com
---
 kernel/watchdog.c |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 75a2ab3..87a19aa 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -518,6 +518,11 @@ int proc_dowatchdog(struct ctl_table *table, int write,
return ret;
 
set_sample_period();
+   /*
+* We shouldn't enable watchdog threads if it is
+* disabled.This is done by watchdog_disabled
+* variable check in watchdog_*_all_cpus function.
+*/
if (watchdog_enabled  watchdog_thresh)
watchdog_enable_all_cpus();
else
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable

[anish kumar]

On Fri, 2013-02-01 at 09:59 -0500, Don Zickus wrote:
> On Fri, Feb 01, 2013 at 07:19:07PM +0530, anish kumar wrote:
> > From: anish kumar 
> > 
> > This watchdog_disabled flag is bit of cryptic.Howerver it's usefullnes is 
> > multifold.
> > Uses are:
> > 1. Check if smpboot_register_percpu_thread function passed.
> > 2. Makes sure that user enables and disables the watchdog in sequence
> >i.e. enable watchdog->disable watchdog->enable watchdog
> >Unlike enable watchdog->enable watchdog which is wrong.
> > 
> > Signed-off-by: anish kumar 
> > ---
> >  kernel/watchdog.c |5 +
> >  1 files changed, 5 insertions(+), 0 deletions(-)
> > 
> > diff --git a/kernel/watchdog.c b/kernel/watchdog.c
> > index 6ef638b..dfd843a 100644
> > --- a/kernel/watchdog.c
> > +++ b/kernel/watchdog.c
> > @@ -519,6 +519,11 @@ int proc_dowatchdog(struct ctl_table *table, int write,
> > return ret;
> >  
> > set_sample_period();
> > +   /*
> > +* We shouldn't enable watchdog threads if it is not
>   ^^^
> the 'not' is not needed I believe.  Other than that, if it helps
> to understand the code better.  I am fine with it.
> 
> Acked-by: Don Zickus 
Should I wait for some more acked-by's?
> 
> > +* disabled.This is done by watchdog_disabled
> > +* variable check in watchdog_*_all_cpus function.
> > +*/
> > if (watchdog_enabled && watchdog_thresh)
> > watchdog_enable_all_cpus();
> > else
> > -- 
> > 1.7.1
> > 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] IRQ CORE: irq_work_queue function return value not used.

[anish kumar]

On Sat, 2013-01-26 at 17:24 +0100, Frederic Weisbecker wrote:
> 2012/11/3 anish kumar :
> > From: anish kumar 
> >
> > As no one is using the return value of irq_work_queue function
> > it is better to just make it void.
> >
> > Acked-by: Steven Rostedt 
> > Signed-off-by: anish kumar 
> > ---
> >  kernel/irq_work.c |5 ++---
> >  1 files changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/kernel/irq_work.c b/kernel/irq_work.c
> > index 1588e3b..4829a31 100644
> > --- a/kernel/irq_work.c
> > +++ b/kernel/irq_work.c
> > @@ -79,17 +79,16 @@ static void __irq_work_queue(struct irq_work *work)
> >   *
> >   * Can be re-enqueued while the callback is still in progress.
> >   */
> > -bool irq_work_queue(struct irq_work *work)
> > +void irq_work_queue(struct irq_work *work)
> 
> Please also update the headers in include/linux/irq_work.h accordingly.
I have sent the patch again(some time back).Is it picked up?
http://www.gossamer-threads.com/lists/linux/kernel/1667796
> 
> Thanks.
> 
> >  {
> > if (!irq_work_claim(work)) {
> > /*
> >  * Already enqueued, can't do!
> >  */
> > -   return false;
> > +   return;
> > }
> >
> > __irq_work_queue(work);
> > -   return true;
> >  }
> >  EXPORT_SYMBOL_GPL(irq_work_queue);
> >
> > --
> > 1.7.1
> >


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable

[anish kumar]

Please ignore this patch.
On Fri, 2013-02-01 at 19:03 +0530, anish kumar wrote:
> From: anish kumar 
> 
> This watchdog_disabled flag is bit of cryptic.Howerver it's usefullnes is 
> multifold.
> Uses are:
> 1. Check if smpboot_register_percpu_thread function passed.
> 2. Makes sure that user enables and disables the watchdog in sequence
>i.e. enable watchdog->disable watchdog->enable watchdog
>Unlike enable watchdog->enable watchdog which is wrong.
> 
> Signed-off-by: anish kumar 
> ---
>  kernel/watchdog.c |1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/watchdog.c b/kernel/watchdog.c
> index 75a2ab3..6ef638b 100644
> --- a/kernel/watchdog.c
> +++ b/kernel/watchdog.c
> @@ -155,6 +155,7 @@ void touch_all_softlockup_watchdogs(void)
>   for_each_online_cpu(cpu)
>   per_cpu(watchdog_touch_ts, cpu) = 0;
>  }
> +EXPORT_SYMBOL(touch_all_softlockup_watchdogs);
>  
>  #ifdef CONFIG_HARDLOCKUP_DETECTOR
>  void touch_nmi_watchdog(void)


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable

[anish kumar]

From: anish kumar 

This watchdog_disabled flag is bit of cryptic.Howerver it's usefullnes is 
multifold.
Uses are:
1. Check if smpboot_register_percpu_thread function passed.
2. Makes sure that user enables and disables the watchdog in sequence
   i.e. enable watchdog->disable watchdog->enable watchdog
   Unlike enable watchdog->enable watchdog which is wrong.

Signed-off-by: anish kumar 
---
 kernel/watchdog.c |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 6ef638b..dfd843a 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -519,6 +519,11 @@ int proc_dowatchdog(struct ctl_table *table, int write,
return ret;
 
set_sample_period();
+   /*
+* We shouldn't enable watchdog threads if it is not
+* disabled.This is done by watchdog_disabled
+* variable check in watchdog_*_all_cpus function.
+*/
if (watchdog_enabled && watchdog_thresh)
watchdog_enable_all_cpus();
else
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable

[anish kumar]

From: anish kumar 

This watchdog_disabled flag is bit of cryptic.Howerver it's usefullnes is 
multifold.
Uses are:
1. Check if smpboot_register_percpu_thread function passed.
2. Makes sure that user enables and disables the watchdog in sequence
   i.e. enable watchdog->disable watchdog->enable watchdog
   Unlike enable watchdog->enable watchdog which is wrong.

Signed-off-by: anish kumar 
---
 kernel/watchdog.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 75a2ab3..6ef638b 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -155,6 +155,7 @@ void touch_all_softlockup_watchdogs(void)
for_each_online_cpu(cpu)
per_cpu(watchdog_touch_ts, cpu) = 0;
 }
+EXPORT_SYMBOL(touch_all_softlockup_watchdogs);
 
 #ifdef CONFIG_HARDLOCKUP_DETECTOR
 void touch_nmi_watchdog(void)
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable

[anish kumar]

From: anish kumar anish198519851...@gmail.com

This watchdog_disabled flag is bit of cryptic.Howerver it's usefullnes is 
multifold.
Uses are:
1. Check if smpboot_register_percpu_thread function passed.
2. Makes sure that user enables and disables the watchdog in sequence
   i.e. enable watchdog-disable watchdog-enable watchdog
   Unlike enable watchdog-enable watchdog which is wrong.

Signed-off-by: anish kumar anish198519851...@gmail.com
---
 kernel/watchdog.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 75a2ab3..6ef638b 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -155,6 +155,7 @@ void touch_all_softlockup_watchdogs(void)
for_each_online_cpu(cpu)
per_cpu(watchdog_touch_ts, cpu) = 0;
 }
+EXPORT_SYMBOL(touch_all_softlockup_watchdogs);
 
 #ifdef CONFIG_HARDLOCKUP_DETECTOR
 void touch_nmi_watchdog(void)
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable

[anish kumar]

From: anish kumar anish198519851...@gmail.com

This watchdog_disabled flag is bit of cryptic.Howerver it's usefullnes is 
multifold.
Uses are:
1. Check if smpboot_register_percpu_thread function passed.
2. Makes sure that user enables and disables the watchdog in sequence
   i.e. enable watchdog-disable watchdog-enable watchdog
   Unlike enable watchdog-enable watchdog which is wrong.

Signed-off-by: anish kumar anish198519851...@gmail.com
---
 kernel/watchdog.c |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index 6ef638b..dfd843a 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -519,6 +519,11 @@ int proc_dowatchdog(struct ctl_table *table, int write,
return ret;
 
set_sample_period();
+   /*
+* We shouldn't enable watchdog threads if it is not
+* disabled.This is done by watchdog_disabled
+* variable check in watchdog_*_all_cpus function.
+*/
if (watchdog_enabled  watchdog_thresh)
watchdog_enable_all_cpus();
else
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable

[anish kumar]

Please ignore this patch.
On Fri, 2013-02-01 at 19:03 +0530, anish kumar wrote:
 From: anish kumar anish198519851...@gmail.com
 
 This watchdog_disabled flag is bit of cryptic.Howerver it's usefullnes is 
 multifold.
 Uses are:
 1. Check if smpboot_register_percpu_thread function passed.
 2. Makes sure that user enables and disables the watchdog in sequence
i.e. enable watchdog-disable watchdog-enable watchdog
Unlike enable watchdog-enable watchdog which is wrong.
 
 Signed-off-by: anish kumar anish198519851...@gmail.com
 ---
  kernel/watchdog.c |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)
 
 diff --git a/kernel/watchdog.c b/kernel/watchdog.c
 index 75a2ab3..6ef638b 100644
 --- a/kernel/watchdog.c
 +++ b/kernel/watchdog.c
 @@ -155,6 +155,7 @@ void touch_all_softlockup_watchdogs(void)
   for_each_online_cpu(cpu)
   per_cpu(watchdog_touch_ts, cpu) = 0;
  }
 +EXPORT_SYMBOL(touch_all_softlockup_watchdogs);
  
  #ifdef CONFIG_HARDLOCKUP_DETECTOR
  void touch_nmi_watchdog(void)


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] IRQ CORE: irq_work_queue function return value not used.

[anish kumar]

On Sat, 2013-01-26 at 17:24 +0100, Frederic Weisbecker wrote:
 2012/11/3 anish kumar anish198519851...@gmail.com:
  From: anish kumar anish198519851...@gmail.com
 
  As no one is using the return value of irq_work_queue function
  it is better to just make it void.
 
  Acked-by: Steven Rostedt rost...@goodmis.org
  Signed-off-by: anish kumar anish198519851...@gmail.com
  ---
   kernel/irq_work.c |5 ++---
   1 files changed, 2 insertions(+), 3 deletions(-)
 
  diff --git a/kernel/irq_work.c b/kernel/irq_work.c
  index 1588e3b..4829a31 100644
  --- a/kernel/irq_work.c
  +++ b/kernel/irq_work.c
  @@ -79,17 +79,16 @@ static void __irq_work_queue(struct irq_work *work)
*
* Can be re-enqueued while the callback is still in progress.
*/
  -bool irq_work_queue(struct irq_work *work)
  +void irq_work_queue(struct irq_work *work)
 
 Please also update the headers in include/linux/irq_work.h accordingly.
I have sent the patch again(some time back).Is it picked up?
http://www.gossamer-threads.com/lists/linux/kernel/1667796
 
 Thanks.
 
   {
  if (!irq_work_claim(work)) {
  /*
   * Already enqueued, can't do!
   */
  -   return false;
  +   return;
  }
 
  __irq_work_queue(work);
  -   return true;
   }
   EXPORT_SYMBOL_GPL(irq_work_queue);
 
  --
  1.7.1
 


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [Watchdog][Trivial] Added comments to explain watchdog_disabled variable

[anish kumar]

On Fri, 2013-02-01 at 09:59 -0500, Don Zickus wrote:
 On Fri, Feb 01, 2013 at 07:19:07PM +0530, anish kumar wrote:
  From: anish kumar anish198519851...@gmail.com
  
  This watchdog_disabled flag is bit of cryptic.Howerver it's usefullnes is 
  multifold.
  Uses are:
  1. Check if smpboot_register_percpu_thread function passed.
  2. Makes sure that user enables and disables the watchdog in sequence
 i.e. enable watchdog-disable watchdog-enable watchdog
 Unlike enable watchdog-enable watchdog which is wrong.
  
  Signed-off-by: anish kumar anish198519851...@gmail.com
  ---
   kernel/watchdog.c |5 +
   1 files changed, 5 insertions(+), 0 deletions(-)
  
  diff --git a/kernel/watchdog.c b/kernel/watchdog.c
  index 6ef638b..dfd843a 100644
  --- a/kernel/watchdog.c
  +++ b/kernel/watchdog.c
  @@ -519,6 +519,11 @@ int proc_dowatchdog(struct ctl_table *table, int write,
  return ret;
   
  set_sample_period();
  +   /*
  +* We shouldn't enable watchdog threads if it is not
   ^^^
 the 'not' is not needed I believe.  Other than that, if it helps
 to understand the code better.  I am fine with it.
 
 Acked-by: Don Zickus dzic...@redhat.com
Should I wait for some more acked-by's?
 
  +* disabled.This is done by watchdog_disabled
  +* variable check in watchdog_*_all_cpus function.
  +*/
  if (watchdog_enabled  watchdog_thresh)
  watchdog_enable_all_cpus();
  else
  -- 
  1.7.1
  


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [BUG?] false positive in soft lockup detector while unlzma initramfs on slow cpu

[anish kumar]

On Wed, 2013-01-30 at 10:51 -0500, Don Zickus wrote:
> On Tue, Jan 29, 2013 at 10:48:27PM +0530, anish kumar wrote:
> > Sorry for digressing from the topic but I think there is something wrong
> > with my understanding or something wrong with the code.So I guess Don
> > can clarify this.
> > If I pass this below parameter during boot i.e. setting watchdog_enabled
> > to zero.
> > __setup("nowatchdog", nowatchdog_setup);
> > 
> > Now I use sysctl to enable the watchdog then wouldn't the below code
> > will hinder enabling the watchdog?
> > 
> > static void watchdog_enable_all_cpus(void)
> > {//snip
> > if (watchdog_disabled) {  /* this is zero ?? */
> > watchdog_disabled = 0;
> > //snip
> > }
> > 
> > Should watchdog_disabled be set to 1?Or is it that we always disable the
> > watchdog and then enable it?
> 
> It seems like a bug, so does something like this fix it?  There is
> probably a better way to handle the internal representation of the
> watchdog state (watchdog_disable) and the procfs version
> (watchdog_enable), but I just can't think of something right now. :-(
> 
> Cheers,
> Don
> 
> 
> diff --git a/kernel/watchdog.c b/kernel/watchdog.c
> index 75a2ab3..d287726 100644
> --- a/kernel/watchdog.c
> +++ b/kernel/watchdog.c
> @@ -82,6 +82,7 @@ __setup("softlockup_panic=", softlockup_panic_setup);
>  static int __init nowatchdog_setup(char *str)
>  {
>   watchdog_enabled = 0;
> + watchdog_disabled =1;
I don't know if this will work or not but while going through the code I
spotted this.I will think of something better if I couldn't think of
anything better than anyway we have this patch.
>   return 1;
>  }
>  __setup("nowatchdog", nowatchdog_setup);


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [BUG?] false positive in soft lockup detector while unlzma initramfs on slow cpu

[anish kumar]

On Wed, 2013-01-30 at 10:51 -0500, Don Zickus wrote:
 On Tue, Jan 29, 2013 at 10:48:27PM +0530, anish kumar wrote:
  Sorry for digressing from the topic but I think there is something wrong
  with my understanding or something wrong with the code.So I guess Don
  can clarify this.
  If I pass this below parameter during boot i.e. setting watchdog_enabled
  to zero.
  __setup(nowatchdog, nowatchdog_setup);
  
  Now I use sysctl to enable the watchdog then wouldn't the below code
  will hinder enabling the watchdog?
  
  static void watchdog_enable_all_cpus(void)
  {//snip
  if (watchdog_disabled) {  /* this is zero ?? */
  watchdog_disabled = 0;
  //snip
  }
  
  Should watchdog_disabled be set to 1?Or is it that we always disable the
  watchdog and then enable it?
 
 It seems like a bug, so does something like this fix it?  There is
 probably a better way to handle the internal representation of the
 watchdog state (watchdog_disable) and the procfs version
 (watchdog_enable), but I just can't think of something right now. :-(
 
 Cheers,
 Don
 
 
 diff --git a/kernel/watchdog.c b/kernel/watchdog.c
 index 75a2ab3..d287726 100644
 --- a/kernel/watchdog.c
 +++ b/kernel/watchdog.c
 @@ -82,6 +82,7 @@ __setup(softlockup_panic=, softlockup_panic_setup);
  static int __init nowatchdog_setup(char *str)
  {
   watchdog_enabled = 0;
 + watchdog_disabled =1;
I don't know if this will work or not but while going through the code I
spotted this.I will think of something better if I couldn't think of
anything better than anyway we have this patch.
   return 1;
  }
  __setup(nowatchdog, nowatchdog_setup);


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [BUG?] false positive in soft lockup detector while unlzma initramfs on slow cpu

[anish kumar]

On Tue, 2013-01-29 at 10:33 -0500, Don Zickus wrote:
> Hi Mike,
> 
> On Tue, Jan 29, 2013 at 05:42:43PM +0400, Mike Lykov wrote:
> > 
> > So my questions:
> > 
> > 1. Are there a BUG in soft lockup detection mechanizm? Changing
> > watchdog_thresh to 30 have a side effect in production - D-state
> > userspace processes will be detected slowly. Are there a need to
> > detecting soft lockups at boot time? Maybe it need after initramfs
you want to disable it then you can do that with nosoftlockup parameter
in kernel-parameters.
> > boot only when userspace processes begin to work?
If my understanding is correct you can do this as well by enabling
watchdog using sysctl.
> 
> The softlockup mechanism works scheduling a high priority task that kicks
> the softlockups.  If the unzip thread is taking too long, it could
> accidentally trip the detection.
> 
> Seeing that you are running on a 600 MHz machine, it could be possible.
> Though I am not entirely sure how the scheduling works for decompressing
> the initramfs.  I wouldn't think it is that high of a priority.
> 
> > 
> > 2. How to change watchdog_thresh parameter at boot without patching
> > sources? If it necessary (with it side effects) maybe implement it
> > as commandline parameter or config compile time parameter?
> 
> I attached a patch below that allows you to set it a boot time.  Let me
> know if this works for you, then I can clean it up and post it properly.
> 
> Cheers,
> Don
> 
> 
> diff --git a/kernel/watchdog.c b/kernel/watchdog.c
> index 75a2ab3..e448d63 100644
> --- a/kernel/watchdog.c
> +++ b/kernel/watchdog.c
> @@ -79,6 +79,14 @@ static int __init softlockup_panic_setup(char *str)
>  }
>  __setup("softlockup_panic=", softlockup_panic_setup);
>  
> +static int __init watchdog_thresh_setup(char *str)
> +{
> + watchdog_thresh = simple_strtoul(str, NULL, 0);
> +
> + return 1;
> +}
> +__setup("watchdog_thresh=", watchdog_thresh_setup);
> +
>  static int __init nowatchdog_setup(char *str)
>  {
>   watchdog_enabled = 0;
Sorry for digressing from the topic but I think there is something wrong
with my understanding or something wrong with the code.So I guess Don
can clarify this.
If I pass this below parameter during boot i.e. setting watchdog_enabled
to zero.
__setup("nowatchdog", nowatchdog_setup);

Now I use sysctl to enable the watchdog then wouldn't the below code
will hinder enabling the watchdog?

static void watchdog_enable_all_cpus(void)
{//snip
if (watchdog_disabled) {  /* this is zero ?? */
watchdog_disabled = 0;
//snip
}

Should watchdog_disabled be set to 1?Or is it that we always disable the
watchdog and then enable it?

This code is used from long time and that is the reason I am puzzled as
to how it can be wrong?I think there is some missing piece which I am
not understanding.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [BUG?] false positive in soft lockup detector while unlzma initramfs on slow cpu

[anish kumar]

On Tue, 2013-01-29 at 10:33 -0500, Don Zickus wrote:
 Hi Mike,
 
 On Tue, Jan 29, 2013 at 05:42:43PM +0400, Mike Lykov wrote:
  
  So my questions:
  
  1. Are there a BUG in soft lockup detection mechanizm? Changing
  watchdog_thresh to 30 have a side effect in production - D-state
  userspace processes will be detected slowly. Are there a need to
  detecting soft lockups at boot time? Maybe it need after initramfs
you want to disable it then you can do that with nosoftlockup parameter
in kernel-parameters.
  boot only when userspace processes begin to work?
If my understanding is correct you can do this as well by enabling
watchdog using sysctl.
 
 The softlockup mechanism works scheduling a high priority task that kicks
 the softlockups.  If the unzip thread is taking too long, it could
 accidentally trip the detection.
 
 Seeing that you are running on a 600 MHz machine, it could be possible.
 Though I am not entirely sure how the scheduling works for decompressing
 the initramfs.  I wouldn't think it is that high of a priority.
 
  
  2. How to change watchdog_thresh parameter at boot without patching
  sources? If it necessary (with it side effects) maybe implement it
  as commandline parameter or config compile time parameter?
 
 I attached a patch below that allows you to set it a boot time.  Let me
 know if this works for you, then I can clean it up and post it properly.
 
 Cheers,
 Don
 
 
 diff --git a/kernel/watchdog.c b/kernel/watchdog.c
 index 75a2ab3..e448d63 100644
 --- a/kernel/watchdog.c
 +++ b/kernel/watchdog.c
 @@ -79,6 +79,14 @@ static int __init softlockup_panic_setup(char *str)
  }
  __setup(softlockup_panic=, softlockup_panic_setup);
  
 +static int __init watchdog_thresh_setup(char *str)
 +{
 + watchdog_thresh = simple_strtoul(str, NULL, 0);
 +
 + return 1;
 +}
 +__setup(watchdog_thresh=, watchdog_thresh_setup);
 +
  static int __init nowatchdog_setup(char *str)
  {
   watchdog_enabled = 0;
Sorry for digressing from the topic but I think there is something wrong
with my understanding or something wrong with the code.So I guess Don
can clarify this.
If I pass this below parameter during boot i.e. setting watchdog_enabled
to zero.
__setup(nowatchdog, nowatchdog_setup);

Now I use sysctl to enable the watchdog then wouldn't the below code
will hinder enabling the watchdog?

static void watchdog_enable_all_cpus(void)
{//snip
if (watchdog_disabled) {  /* this is zero ?? */
watchdog_disabled = 0;
//snip
}

Should watchdog_disabled be set to 1?Or is it that we always disable the
watchdog and then enable it?

This code is used from long time and that is the reason I am puzzled as
to how it can be wrong?I think there is some missing piece which I am
not understanding.
 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [PATCH] IRQ CORE: irq_work_queue function return value not used.

[anish kumar]

From: anish kumar 

As no one is using the return value of irq_work_queue function
it is better to just make it void.

Acked-by: Steven Rostedt 
Signed-off-by: anish kumar 
---
 include/linux/irq_work.h |2 +-
 kernel/irq_work.c|5 ++---
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/include/linux/irq_work.h b/include/linux/irq_work.h
index 6a9e8f5..ce60c08 100644
--- a/include/linux/irq_work.h
+++ b/include/linux/irq_work.h
@@ -16,7 +16,7 @@ void init_irq_work(struct irq_work *work, void (*func)(struct 
irq_work *))
work->func = func;
 }
 
-bool irq_work_queue(struct irq_work *work);
+void irq_work_queue(struct irq_work *work);
 void irq_work_run(void);
 void irq_work_sync(struct irq_work *work);
 
diff --git a/kernel/irq_work.c b/kernel/irq_work.c
index 1588e3b..4829a31 100644
--- a/kernel/irq_work.c
+++ b/kernel/irq_work.c
@@ -79,17 +79,16 @@ static void __irq_work_queue(struct irq_work *work)
  *
  * Can be re-enqueued while the callback is still in progress.
  */
-bool irq_work_queue(struct irq_work *work)
+void irq_work_queue(struct irq_work *work)
 {
if (!irq_work_claim(work)) {
/*
 * Already enqueued, can't do!
 */
-   return false;
+   return;
}
 
__irq_work_queue(work);
-   return true;
 }
 EXPORT_SYMBOL_GPL(irq_work_queue);
 
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [PATCH] IRQ CORE: irq_work_queue function return value not used.

[anish kumar]

From: anish kumar anish198519851...@gmail.com

As no one is using the return value of irq_work_queue function
it is better to just make it void.

Acked-by: Steven Rostedt rost...@goodmis.org
Signed-off-by: anish kumar anish198519851...@gmail.com
---
 include/linux/irq_work.h |2 +-
 kernel/irq_work.c|5 ++---
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/include/linux/irq_work.h b/include/linux/irq_work.h
index 6a9e8f5..ce60c08 100644
--- a/include/linux/irq_work.h
+++ b/include/linux/irq_work.h
@@ -16,7 +16,7 @@ void init_irq_work(struct irq_work *work, void (*func)(struct 
irq_work *))
work-func = func;
 }
 
-bool irq_work_queue(struct irq_work *work);
+void irq_work_queue(struct irq_work *work);
 void irq_work_run(void);
 void irq_work_sync(struct irq_work *work);
 
diff --git a/kernel/irq_work.c b/kernel/irq_work.c
index 1588e3b..4829a31 100644
--- a/kernel/irq_work.c
+++ b/kernel/irq_work.c
@@ -79,17 +79,16 @@ static void __irq_work_queue(struct irq_work *work)
  *
  * Can be re-enqueued while the callback is still in progress.
  */
-bool irq_work_queue(struct irq_work *work)
+void irq_work_queue(struct irq_work *work)
 {
if (!irq_work_claim(work)) {
/*
 * Already enqueued, can't do!
 */
-   return false;
+   return;
}
 
__irq_work_queue(work);
-   return true;
 }
 EXPORT_SYMBOL_GPL(irq_work_queue);
 
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: what is the function of do_softirq() ?

[anish kumar]

On Wed, 2013-01-16 at 10:25 +, Anuz Pratap Singh Tomar wrote:
> 
> 
> On Tue, Jan 15, 2013 at 6:31 AM, horseriver 
> wrote:
> hi:
> 
>what is the function of do_softirq()?
Softirq is basically same as bottom half except it is run in irq
context.So the question which comes to mind is why softirq?Softirqs can
run concurrently on several CPUs and that is why it used in networking.
There are other advantages also but it is mostly use case dependent.
> 
>It is called by ksoftirqd() ,which is setup by :
> kernel_thread(ksoftirqd, hcpu, CLONE_KERNEL) ; 
ksoftirq is a saviour thread which takes up the execution of softirq if
it finds out that softirq are executing one by one and thereby userspace
is not being scheduled or none of other task is getting executed.As
threads have low priority it lets other tasks run.
> 
> Please read "Understanding the Linux Kernel" Chapter on Interrupts and
> Section on Softirqs and tasklets.  Page number 171(might be different
> in other editions) onwards.
> 
> 
> 
> thanks!
> 
> ___
> Kernelnewbies mailing list
> kernelnewb...@kernelnewbies.org
> http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
> 
> 
> 
> -- 
> Thank you 
> Warm Regards
> Anuz
> ___
> Kernelnewbies mailing list
> kernelnewb...@kernelnewbies.org
> http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: what is the function of do_softirq() ?

[anish kumar]

On Wed, 2013-01-16 at 10:25 +, Anuz Pratap Singh Tomar wrote:
 
 
 On Tue, Jan 15, 2013 at 6:31 AM, horseriver horseriv...@gmail.com
 wrote:
 hi:
 
what is the function of do_softirq()?
Softirq is basically same as bottom half except it is run in irq
context.So the question which comes to mind is why softirq?Softirqs can
run concurrently on several CPUs and that is why it used in networking.
There are other advantages also but it is mostly use case dependent.
 
It is called by ksoftirqd() ,which is setup by :
 kernel_thread(ksoftirqd, hcpu, CLONE_KERNEL) ; 
ksoftirq is a saviour thread which takes up the execution of softirq if
it finds out that softirq are executing one by one and thereby userspace
is not being scheduled or none of other task is getting executed.As
threads have low priority it lets other tasks run.
 
 Please read Understanding the Linux Kernel Chapter on Interrupts and
 Section on Softirqs and tasklets.  Page number 171(might be different
 in other editions) onwards.
 
 
 
 thanks!
 
 ___
 Kernelnewbies mailing list
 kernelnewb...@kernelnewbies.org
 http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
 
 
 
 -- 
 Thank you 
 Warm Regards
 Anuz
 ___
 Kernelnewbies mailing list
 kernelnewb...@kernelnewbies.org
 http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

watchdog code

[anish kumar]

>From your comments in this thread https://lkml.org/lkml/2011/3/25/723 

>The msm watchdog driver is present in kernel only. It does not use the
>built-in Linux watchdog api. This is because the primary function of
>our watchdog is detecting bus lockups and interrupts being turned off
Doesn't linux original implementation(kernel/watchdog.c) already cover
this?If not then how does this implementation detect it i.e. bus lockup
and interrupt turned off for long time?
Does this piece of code can co-exist with the soft/hard lockup detection
in the core kernel?
>for long periods of time. We wanted this functionality to be present
>regardless of the userspace the kernel is running beneath. Userspace is
>free to have its own watchdog implemented in software.
what does this mean, can you elaborate?

>Signed-off-by: Jeff Ohlstein 
In my personal opinion, we should always acknowledge the code from which
this code is inspired :)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

watchdog code

[anish kumar]

From your comments in this thread https://lkml.org/lkml/2011/3/25/723 

The msm watchdog driver is present in kernel only. It does not use the
built-in Linux watchdog api. This is because the primary function of
our watchdog is detecting bus lockups and interrupts being turned off
Doesn't linux original implementation(kernel/watchdog.c) already cover
this?If not then how does this implementation detect it i.e. bus lockup
and interrupt turned off for long time?
Does this piece of code can co-exist with the soft/hard lockup detection
in the core kernel?
for long periods of time. We wanted this functionality to be present
regardless of the userspace the kernel is running beneath. Userspace is
free to have its own watchdog implemented in software.
what does this mean, can you elaborate?

Signed-off-by: Jeff Ohlstein johls...@codeaurora.org
In my personal opinion, we should always acknowledge the code from which
this code is inspired :)

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: How kernel handle interrupts[AX88796B network controller]

[anish kumar]

On Sat, 2012-12-22 at 23:11 +0800, Woody Wu wrote:
> On Fri, Dec 21, 2012 at 01:33:03PM -0800, anish kumar wrote:
> > On Fri, 2012-12-21 at 23:34 +0800, Woody Wu wrote:
> > > On Thu, Dec 20, 2012 at 10:05:05AM -0800, anish singh wrote:
> > > > On Dec 20, 2012 6:30 AM, "Woody Wu"  wrote:
> > > > >
> > > > > Hi, List
> > > > >
> > > > > Where is the Kernel code that handles external interrupts? I want to
> > > > > have a look at it but haven't found out where it is.
> > > > >
> > > > > Actually, I have some basic questions about interrupt handling in 
> > > > > Linux.
> > > > > 1. After Kernel's ISR received an interrupt, I believe it will invoke 
> > > > > a
> > > > >handler defined in a device driver if any. But it should be the
> > > > >device driver's responsibility or kernel ISR's responsibility to
> > > > >clear (or acknowledge) the interrupt?
> > > > If the interrupt in question is currently being handled then in
> > > > the case of edge triggered interrupt we just mask the interrupt,set it
> > > > pending and bail out.Once the interrupt handler completes then we check 
> > > > for
> > > > pending interrupt and handle it.In level triggered we don't do that.
> > > > Kerenel ISR -this is mixture of core kernel interrupt handling code + 
> > > > your
> > > > device driver interrupt handler(if this is chip driver which is 
> > > > supposed to
> > > > get one interrupt and is reponsible for calling other interrupt handlers
> > > > based on the chip register status then you do explicit masking unmasking
> > > > yourself).
> > > > If you device driver is a interrupt controller driver then you register
> > > > your driver with kernel interrupt handling code and need to write some
> > > > callbacks such as .mask,.unmask and so on.This callbacks are called at
> > > > appropiate places whenever the interrupt is raised.This interrupt is 
> > > > then
> > > > passed to drivers who has requested for this interrupt by calling
> > > > request_irq.
> > > > >
> > > > > 2. My device, an AX88796B network controller, asserting the interrupt
> > > > >line in a level-triggered manner. Now I met problem with the device
> > > > that
> > > > >might caused by the CPU interrupt mode is not set as 
> > > > > level-triggered by
> > > > >edge trigger.  My CPU is Samsung S3C2410, an ARM920T powered one.  
> > > > > Does
> > > > >anyone know usually where and how should I do this kind of setting?
> > > > Just pass the parameter "level triggered" in request_irq in your device
> > > > driver.
> > > 
> > > Hi Sign,
> > > 
> > > I searched the interrupt.h for the all the defined flags that I can pass
> > > to the request_irq, but there is no a flag looks like "level triggered".
> > > Would you tell me what you mean the parameter "level triggered"?
> > irq_set_irq_type(info->irq, IRQ_TYPE_LEVEL_LOW)
> > 
> > include/linux/irq.h
> > IRQ_TYPE_LEVEL_HIGH  - high level triggered
> > IRQ_TYPE_LEVEL_LOW   - low level triggered
> 
> Thanks. Now I find the function.
> 
> I searched some code about irq in ARM architecure.  Some other
> people talked about do_IRQ() probabaly is wrong for ARM. There is simply
> no that function in ARM. Maybe the do_IRQ in x86 is replaced by
> handle_IRQ.
arch/arm/kernel/entry-armv.S
__irq_svc is called by the arm processor which in turn calls irq_handler
macro.I think this is the lowest level handler after which linux
interrupt handling takes over.
> 
> For the irq_set_irq_type(), do you think what's the correct place to
> call it? Inside my device driver or outside the device driver (probably
> in the board definition file)? If that should be called inside a device
> driver, should it be the driver probe function or in the open function?
> After or before the invocation of request_irq()?
irq_set_irq_type() should be called by device driver code not by the
board file.It should be called in the probe function AFAIK.
> 
> Sorry for asking too many question.  I found the kernel + device driver
> irq handling part still not clear to me.
You are welcome to ask as many question as you want.
> 
> 
> > > 
> > > Thanks.
> > > 
> > > > >
> > > > >
> > > > > Thanks in advance.
> > > > >
> > > > > --
> > > > > woody
> > > > > I can't go back to yesterday - because I was a different person then.
> > > > >
> > > > > ___
> > > > > Kernelnewbies mailing list
> > > > > kernelnewb...@kernelnewbies.org
> > > > > http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
> > > 
> > 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: How kernel handle interrupts[AX88796B network controller]

[anish kumar]

On Mon, 2012-12-24 at 22:10 +0800, Woody Wu wrote:
> On Fri, Dec 21, 2012 at 01:33:03PM -0800, anish kumar wrote:
> > On Fri, 2012-12-21 at 23:34 +0800, Woody Wu wrote:
> > > On Thu, Dec 20, 2012 at 10:05:05AM -0800, anish singh wrote:
> > > > On Dec 20, 2012 6:30 AM, "Woody Wu"  wrote:
> > > > >
> > > > > Hi, List
> > > > >
> > > > > Where is the Kernel code that handles external interrupts? I want to
> > > > > have a look at it but haven't found out where it is.
> > > > >
> > > > > Actually, I have some basic questions about interrupt handling in 
> > > > > Linux.
> > > > > 1. After Kernel's ISR received an interrupt, I believe it will invoke 
> > > > > a
> > > > >handler defined in a device driver if any. But it should be the
> > > > >device driver's responsibility or kernel ISR's responsibility to
> > > > >clear (or acknowledge) the interrupt?
> > > > If the interrupt in question is currently being handled then in
> > > > the case of edge triggered interrupt we just mask the interrupt,set it
> > > > pending and bail out.Once the interrupt handler completes then we check 
> > > > for
> > > > pending interrupt and handle it.In level triggered we don't do that.
> > > > Kerenel ISR -this is mixture of core kernel interrupt handling code + 
> > > > your
> > > > device driver interrupt handler(if this is chip driver which is 
> > > > supposed to
> > > > get one interrupt and is reponsible for calling other interrupt handlers
> > > > based on the chip register status then you do explicit masking unmasking
> > > > yourself).
> > > > If you device driver is a interrupt controller driver then you register
> > > > your driver with kernel interrupt handling code and need to write some
> > > > callbacks such as .mask,.unmask and so on.This callbacks are called at
> > > > appropiate places whenever the interrupt is raised.This interrupt is 
> > > > then
> > > > passed to drivers who has requested for this interrupt by calling
> > > > request_irq.
> > > > >
> > > > > 2. My device, an AX88796B network controller, asserting the interrupt
> > > > >line in a level-triggered manner. Now I met problem with the device
> > > > that
> > > > >might caused by the CPU interrupt mode is not set as 
> > > > > level-triggered by
> > > > >edge trigger.  My CPU is Samsung S3C2410, an ARM920T powered one.  
> > > > > Does
> > > > >anyone know usually where and how should I do this kind of setting?
> > > > Just pass the parameter "level triggered" in request_irq in your device
> > > > driver.
> > > 
> > > Hi Sign,
> > > 
> > > I searched the interrupt.h for the all the defined flags that I can pass
> > > to the request_irq, but there is no a flag looks like "level triggered".
> > > Would you tell me what you mean the parameter "level triggered"?
> > irq_set_irq_type(info->irq, IRQ_TYPE_LEVEL_LOW)
> > 
> > include/linux/irq.h
> > IRQ_TYPE_LEVEL_HIGH  - high level triggered
> > IRQ_TYPE_LEVEL_LOW   - low level triggered
> 
> Thanks. You saved my ass.
> 
> Be curious, I found the api changes from 2.6 to 3.7.  In 2.6, there are
> pair of funtions, set_irq_type and set_irq_handle (there is no
> irq_set_irq_type in 2.6).  Problem is, I cannot find something like
> irq_set_irq_handle in 3.7.  Does that mean, in 3.7, when
> irq_set_irq_type is changed, the associated flow handler is also
> changed?  In my case, the interrupt was originally assgined with a edge
> flow handler and set type as edge irq. After I, by invoking
> irq_set_irq_type, change it to level irq, I think the flow handler
> should also be changed to a level handle.  Is that happened
> automatically behind?  I search through the code, but did not find where
> is it.
Why not try calling irq_set_irq_type and check what happens?
> 
> 
> > > 
> > > Thanks.
> > > 
> > > > >
> > > > >
> > > > > Thanks in advance.
> > > > >
> > > > > --
> > > > > woody
> > > > > I can't go back to yesterday - because I was a different person then.
> > > > >
> > > > > ___
> > > > > Kernelnewbies mailing list
> > > > > kernelnewb...@kernelnewbies.org
> > > > > http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
> > > 
> > 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: How kernel handle interrupts[AX88796B network controller]

[anish kumar]

On Mon, 2012-12-24 at 22:10 +0800, Woody Wu wrote:
 On Fri, Dec 21, 2012 at 01:33:03PM -0800, anish kumar wrote:
  On Fri, 2012-12-21 at 23:34 +0800, Woody Wu wrote:
   On Thu, Dec 20, 2012 at 10:05:05AM -0800, anish singh wrote:
On Dec 20, 2012 6:30 AM, Woody Wu narkewo...@gmail.com wrote:

 Hi, List

 Where is the Kernel code that handles external interrupts? I want to
 have a look at it but haven't found out where it is.

 Actually, I have some basic questions about interrupt handling in 
 Linux.
 1. After Kernel's ISR received an interrupt, I believe it will invoke 
 a
handler defined in a device driver if any. But it should be the
device driver's responsibility or kernel ISR's responsibility to
clear (or acknowledge) the interrupt?
If the interrupt in question is currently being handled then in
the case of edge triggered interrupt we just mask the interrupt,set it
pending and bail out.Once the interrupt handler completes then we check 
for
pending interrupt and handle it.In level triggered we don't do that.
Kerenel ISR -this is mixture of core kernel interrupt handling code + 
your
device driver interrupt handler(if this is chip driver which is 
supposed to
get one interrupt and is reponsible for calling other interrupt handlers
based on the chip register status then you do explicit masking unmasking
yourself).
If you device driver is a interrupt controller driver then you register
your driver with kernel interrupt handling code and need to write some
callbacks such as .mask,.unmask and so on.This callbacks are called at
appropiate places whenever the interrupt is raised.This interrupt is 
then
passed to drivers who has requested for this interrupt by calling
request_irq.

 2. My device, an AX88796B network controller, asserting the interrupt
line in a level-triggered manner. Now I met problem with the device
that
might caused by the CPU interrupt mode is not set as 
 level-triggered by
edge trigger.  My CPU is Samsung S3C2410, an ARM920T powered one.  
 Does
anyone know usually where and how should I do this kind of setting?
Just pass the parameter level triggered in request_irq in your device
driver.
   
   Hi Sign,
   
   I searched the interrupt.h for the all the defined flags that I can pass
   to the request_irq, but there is no a flag looks like level triggered.
   Would you tell me what you mean the parameter level triggered?
  irq_set_irq_type(info-irq, IRQ_TYPE_LEVEL_LOW)
  
  include/linux/irq.h
  IRQ_TYPE_LEVEL_HIGH  - high level triggered
  IRQ_TYPE_LEVEL_LOW   - low level triggered
 
 Thanks. You saved my ass.
 
 Be curious, I found the api changes from 2.6 to 3.7.  In 2.6, there are
 pair of funtions, set_irq_type and set_irq_handle (there is no
 irq_set_irq_type in 2.6).  Problem is, I cannot find something like
 irq_set_irq_handle in 3.7.  Does that mean, in 3.7, when
 irq_set_irq_type is changed, the associated flow handler is also
 changed?  In my case, the interrupt was originally assgined with a edge
 flow handler and set type as edge irq. After I, by invoking
 irq_set_irq_type, change it to level irq, I think the flow handler
 should also be changed to a level handle.  Is that happened
 automatically behind?  I search through the code, but did not find where
 is it.
Why not try calling irq_set_irq_type and check what happens?
 
 
   
   Thanks.
   


 Thanks in advance.

 --
 woody
 I can't go back to yesterday - because I was a different person then.

 ___
 Kernelnewbies mailing list
 kernelnewb...@kernelnewbies.org
 http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
   
  
 


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: How kernel handle interrupts[AX88796B network controller]

[anish kumar]

On Sat, 2012-12-22 at 23:11 +0800, Woody Wu wrote:
 On Fri, Dec 21, 2012 at 01:33:03PM -0800, anish kumar wrote:
  On Fri, 2012-12-21 at 23:34 +0800, Woody Wu wrote:
   On Thu, Dec 20, 2012 at 10:05:05AM -0800, anish singh wrote:
On Dec 20, 2012 6:30 AM, Woody Wu narkewo...@gmail.com wrote:

 Hi, List

 Where is the Kernel code that handles external interrupts? I want to
 have a look at it but haven't found out where it is.

 Actually, I have some basic questions about interrupt handling in 
 Linux.
 1. After Kernel's ISR received an interrupt, I believe it will invoke 
 a
handler defined in a device driver if any. But it should be the
device driver's responsibility or kernel ISR's responsibility to
clear (or acknowledge) the interrupt?
If the interrupt in question is currently being handled then in
the case of edge triggered interrupt we just mask the interrupt,set it
pending and bail out.Once the interrupt handler completes then we check 
for
pending interrupt and handle it.In level triggered we don't do that.
Kerenel ISR -this is mixture of core kernel interrupt handling code + 
your
device driver interrupt handler(if this is chip driver which is 
supposed to
get one interrupt and is reponsible for calling other interrupt handlers
based on the chip register status then you do explicit masking unmasking
yourself).
If you device driver is a interrupt controller driver then you register
your driver with kernel interrupt handling code and need to write some
callbacks such as .mask,.unmask and so on.This callbacks are called at
appropiate places whenever the interrupt is raised.This interrupt is 
then
passed to drivers who has requested for this interrupt by calling
request_irq.

 2. My device, an AX88796B network controller, asserting the interrupt
line in a level-triggered manner. Now I met problem with the device
that
might caused by the CPU interrupt mode is not set as 
 level-triggered by
edge trigger.  My CPU is Samsung S3C2410, an ARM920T powered one.  
 Does
anyone know usually where and how should I do this kind of setting?
Just pass the parameter level triggered in request_irq in your device
driver.
   
   Hi Sign,
   
   I searched the interrupt.h for the all the defined flags that I can pass
   to the request_irq, but there is no a flag looks like level triggered.
   Would you tell me what you mean the parameter level triggered?
  irq_set_irq_type(info-irq, IRQ_TYPE_LEVEL_LOW)
  
  include/linux/irq.h
  IRQ_TYPE_LEVEL_HIGH  - high level triggered
  IRQ_TYPE_LEVEL_LOW   - low level triggered
 
 Thanks. Now I find the function.
 
 I searched some code about irq in ARM architecure.  Some other
 people talked about do_IRQ() probabaly is wrong for ARM. There is simply
 no that function in ARM. Maybe the do_IRQ in x86 is replaced by
 handle_IRQ.
arch/arm/kernel/entry-armv.S
__irq_svc is called by the arm processor which in turn calls irq_handler
macro.I think this is the lowest level handler after which linux
interrupt handling takes over.
 
 For the irq_set_irq_type(), do you think what's the correct place to
 call it? Inside my device driver or outside the device driver (probably
 in the board definition file)? If that should be called inside a device
 driver, should it be the driver probe function or in the open function?
 After or before the invocation of request_irq()?
irq_set_irq_type() should be called by device driver code not by the
board file.It should be called in the probe function AFAIK.
 
 Sorry for asking too many question.  I found the kernel + device driver
 irq handling part still not clear to me.
You are welcome to ask as many question as you want.
 
 
   
   Thanks.
   


 Thanks in advance.

 --
 woody
 I can't go back to yesterday - because I was a different person then.

 ___
 Kernelnewbies mailing list
 kernelnewb...@kernelnewbies.org
 http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
   
  
 


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: What does ISA/PCI really mean to ARM architecture?

[anish kumar]

On Thu, 2012-12-27 at 11:22 -0500, jonsm...@gmail.com wrote:
> On Thu, Dec 27, 2012 at 3:27 AM, Woody Wu  wrote:
> > Hi, list
> >
> > I know this might be a very basic question.  But I really don't clear at
> > it.
> >
> > Can a peripheral chip that claims to be ISA or PCI device be used in a
> > ARM based embedded system?  For these kind of chips, I only concern
> > about the planar kind of devices, means they are not on a dedicated
> > expansion card.
> >
> > From hardware point of view, to attach a ISA or PCI planar chip, is
> > there any requirement need to fulfill on a ARM board?
> 
> See if your ARM CPU has an interface for SRAM (in addition to DRAM).
> You can use a SRAM chip select to access ISA type devices. But you may
Would you mind explaining this in detail?
> need additional buffers/latches to do this.
> 
> Another solution is to attach you peripherals using USB. Almost all
Connect using USB what does this mean?
> embedded wifi chips are attached this way. The USB connectors aren't
> required, you can route USB around on your PCB. USB hub chips are
> $0.35 if you need more ports.  USB Ethernet chips are available.
> 
> Other options include SPI/I2C. It is worthwhile to investigate these
Only chips which support SPI/I2C can be used but ISA/PCI is completely
orthogonal to this AFAIK.
> serial solutions before doing a parallel solution. Parallel buses eat
> up a lot of PCB space.
> 
> 
> >
> > From Linux driver point of view, what are needed to support an ISA or
> > PCI driver in ARM architecture?  More important, is ISA or PCI device a
> > platform device?  If not, how to add these kind of devices in my board
> > definition?
> >
> > I know my question might not be reasonable enough, if I messed concepts,
> > please sort me out.
> >
> >
> > Thanks in advance.
> >
> >
> > --
> > woody
> > I can't go back to yesterday - because I was a different person then.
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> 
> 
> 
> --
> Jon Smirl
> jonsm...@gmail.com
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: What does ISA/PCI really mean to ARM architecture?

[anish kumar]

On Thu, 2012-12-27 at 10:51 +0100, Geert Uytterhoeven wrote:
> On Thu, Dec 27, 2012 at 9:27 AM, Woody Wu  wrote:
> > Can a peripheral chip that claims to be ISA or PCI device be used in a
> > ARM based embedded system?  For these kind of chips, I only concern
> > about the planar kind of devices, means they are not on a dedicated
> > expansion card.
> >
> > From hardware point of view, to attach a ISA or PCI planar chip, is
> > there any requirement need to fulfill on a ARM bard?
arm AFAIK is only used in embedded system but ISA/PCI buses are
generally part of 'big systems' and most of the times it refers to x86
PC.
> >
> > From Linux driver point of view, what are needed to support an ISA or
> > PCI driver in ARM architecture?  More important, is ISA or PCI device a
> > platform device?  If not, how to add these kind of devices in my board
> > definition?
AFAIK, Platform device is just a way to add a particular driver whose
probe can't be called at runtime.Mostly platform device is part of
system on chip.
PCI devices probe function will be called by the PCI bus as and when it
detects any activity on the bus.So you don't need PCI device to be  a
platform device.
> 
> An ISA device is typically a platform device. For ARM, which uses device 
> trees,
Don't know much about ISA device to comment on this but people familiar
with this can enlighten us as to the reason why it is platform device in
detail.
> this means you define it in the device tree.
> 
> A PCI device is not a platform device, as devices on a PCI bus can be
> probed automatically. The PCI host bridge is typically a platform device,
> though, so it it should be in your device tree.
> 
> Gr{oetje,eeting}s,
> 
> Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
> ge...@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like 
> that.
> -- Linus Torvalds
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: What does ISA/PCI really mean to ARM architecture?

[anish kumar]

On Thu, 2012-12-27 at 10:51 +0100, Geert Uytterhoeven wrote:
 On Thu, Dec 27, 2012 at 9:27 AM, Woody Wu narkewo...@gmail.com wrote:
  Can a peripheral chip that claims to be ISA or PCI device be used in a
  ARM based embedded system?  For these kind of chips, I only concern
  about the planar kind of devices, means they are not on a dedicated
  expansion card.
 
  From hardware point of view, to attach a ISA or PCI planar chip, is
  there any requirement need to fulfill on a ARM bard?
arm AFAIK is only used in embedded system but ISA/PCI buses are
generally part of 'big systems' and most of the times it refers to x86
PC.
 
  From Linux driver point of view, what are needed to support an ISA or
  PCI driver in ARM architecture?  More important, is ISA or PCI device a
  platform device?  If not, how to add these kind of devices in my board
  definition?
AFAIK, Platform device is just a way to add a particular driver whose
probe can't be called at runtime.Mostly platform device is part of
system on chip.
PCI devices probe function will be called by the PCI bus as and when it
detects any activity on the bus.So you don't need PCI device to be  a
platform device.
 
 An ISA device is typically a platform device. For ARM, which uses device 
 trees,
Don't know much about ISA device to comment on this but people familiar
with this can enlighten us as to the reason why it is platform device in
detail.
 this means you define it in the device tree.
 
 A PCI device is not a platform device, as devices on a PCI bus can be
 probed automatically. The PCI host bridge is typically a platform device,
 though, so it it should be in your device tree.
 
 Gr{oetje,eeting}s,
 
 Geert
 
 --
 Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
 ge...@linux-m68k.org
 
 In personal conversations with technical people, I call myself a hacker. But
 when I'm talking to journalists I just say programmer or something like 
 that.
 -- Linus Torvalds
 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: What does ISA/PCI really mean to ARM architecture?

[anish kumar]

On Thu, 2012-12-27 at 11:22 -0500, jonsm...@gmail.com wrote:
 On Thu, Dec 27, 2012 at 3:27 AM, Woody Wu narkewo...@gmail.com wrote:
  Hi, list
 
  I know this might be a very basic question.  But I really don't clear at
  it.
 
  Can a peripheral chip that claims to be ISA or PCI device be used in a
  ARM based embedded system?  For these kind of chips, I only concern
  about the planar kind of devices, means they are not on a dedicated
  expansion card.
 
  From hardware point of view, to attach a ISA or PCI planar chip, is
  there any requirement need to fulfill on a ARM board?
 
 See if your ARM CPU has an interface for SRAM (in addition to DRAM).
 You can use a SRAM chip select to access ISA type devices. But you may
Would you mind explaining this in detail?
 need additional buffers/latches to do this.
 
 Another solution is to attach you peripherals using USB. Almost all
Connect using USB what does this mean?
 embedded wifi chips are attached this way. The USB connectors aren't
 required, you can route USB around on your PCB. USB hub chips are
 $0.35 if you need more ports.  USB Ethernet chips are available.
 
 Other options include SPI/I2C. It is worthwhile to investigate these
Only chips which support SPI/I2C can be used but ISA/PCI is completely
orthogonal to this AFAIK.
 serial solutions before doing a parallel solution. Parallel buses eat
 up a lot of PCB space.
 
 
 
  From Linux driver point of view, what are needed to support an ISA or
  PCI driver in ARM architecture?  More important, is ISA or PCI device a
  platform device?  If not, how to add these kind of devices in my board
  definition?
 
  I know my question might not be reasonable enough, if I messed concepts,
  please sort me out.
 
 
  Thanks in advance.
 
 
  --
  woody
  I can't go back to yesterday - because I was a different person then.
  --
  To unsubscribe from this list: send the line unsubscribe linux-kernel in
  the body of a message to majord...@vger.kernel.org
  More majordomo info at  http://vger.kernel.org/majordomo-info.html
  Please read the FAQ at  http://www.tux.org/lkml/
 
 
 
 --
 Jon Smirl
 jonsm...@gmail.com
 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: How kernel handle interrupts[AX88796B network controller]

[anish kumar]

On Fri, 2012-12-21 at 23:34 +0800, Woody Wu wrote:
> On Thu, Dec 20, 2012 at 10:05:05AM -0800, anish singh wrote:
> > On Dec 20, 2012 6:30 AM, "Woody Wu"  wrote:
> > >
> > > Hi, List
> > >
> > > Where is the Kernel code that handles external interrupts? I want to
> > > have a look at it but haven't found out where it is.
> > >
> > > Actually, I have some basic questions about interrupt handling in Linux.
> > > 1. After Kernel's ISR received an interrupt, I believe it will invoke a
> > >handler defined in a device driver if any. But it should be the
> > >device driver's responsibility or kernel ISR's responsibility to
> > >clear (or acknowledge) the interrupt?
> > If the interrupt in question is currently being handled then in
> > the case of edge triggered interrupt we just mask the interrupt,set it
> > pending and bail out.Once the interrupt handler completes then we check for
> > pending interrupt and handle it.In level triggered we don't do that.
> > Kerenel ISR -this is mixture of core kernel interrupt handling code + your
> > device driver interrupt handler(if this is chip driver which is supposed to
> > get one interrupt and is reponsible for calling other interrupt handlers
> > based on the chip register status then you do explicit masking unmasking
> > yourself).
> > If you device driver is a interrupt controller driver then you register
> > your driver with kernel interrupt handling code and need to write some
> > callbacks such as .mask,.unmask and so on.This callbacks are called at
> > appropiate places whenever the interrupt is raised.This interrupt is then
> > passed to drivers who has requested for this interrupt by calling
> > request_irq.
> > >
> > > 2. My device, an AX88796B network controller, asserting the interrupt
> > >line in a level-triggered manner. Now I met problem with the device
> > that
> > >might caused by the CPU interrupt mode is not set as level-triggered by
> > >edge trigger.  My CPU is Samsung S3C2410, an ARM920T powered one.  Does
> > >anyone know usually where and how should I do this kind of setting?
> > Just pass the parameter "level triggered" in request_irq in your device
> > driver.
> 
> Hi Sign,
> 
> I searched the interrupt.h for the all the defined flags that I can pass
> to the request_irq, but there is no a flag looks like "level triggered".
> Would you tell me what you mean the parameter "level triggered"?
irq_set_irq_type(info->irq, IRQ_TYPE_LEVEL_LOW)

include/linux/irq.h
IRQ_TYPE_LEVEL_HIGH  - high level triggered
IRQ_TYPE_LEVEL_LOW   - low level triggered
> 
> Thanks.
> 
> > >
> > >
> > > Thanks in advance.
> > >
> > > --
> > > woody
> > > I can't go back to yesterday - because I was a different person then.
> > >
> > > ___
> > > Kernelnewbies mailing list
> > > kernelnewb...@kernelnewbies.org
> > > http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: How kernel handle interrupts[AX88796B network controller]

[anish kumar]

On Fri, 2012-12-21 at 23:34 +0800, Woody Wu wrote:
 On Thu, Dec 20, 2012 at 10:05:05AM -0800, anish singh wrote:
  On Dec 20, 2012 6:30 AM, Woody Wu narkewo...@gmail.com wrote:
  
   Hi, List
  
   Where is the Kernel code that handles external interrupts? I want to
   have a look at it but haven't found out where it is.
  
   Actually, I have some basic questions about interrupt handling in Linux.
   1. After Kernel's ISR received an interrupt, I believe it will invoke a
  handler defined in a device driver if any. But it should be the
  device driver's responsibility or kernel ISR's responsibility to
  clear (or acknowledge) the interrupt?
  If the interrupt in question is currently being handled then in
  the case of edge triggered interrupt we just mask the interrupt,set it
  pending and bail out.Once the interrupt handler completes then we check for
  pending interrupt and handle it.In level triggered we don't do that.
  Kerenel ISR -this is mixture of core kernel interrupt handling code + your
  device driver interrupt handler(if this is chip driver which is supposed to
  get one interrupt and is reponsible for calling other interrupt handlers
  based on the chip register status then you do explicit masking unmasking
  yourself).
  If you device driver is a interrupt controller driver then you register
  your driver with kernel interrupt handling code and need to write some
  callbacks such as .mask,.unmask and so on.This callbacks are called at
  appropiate places whenever the interrupt is raised.This interrupt is then
  passed to drivers who has requested for this interrupt by calling
  request_irq.
  
   2. My device, an AX88796B network controller, asserting the interrupt
  line in a level-triggered manner. Now I met problem with the device
  that
  might caused by the CPU interrupt mode is not set as level-triggered by
  edge trigger.  My CPU is Samsung S3C2410, an ARM920T powered one.  Does
  anyone know usually where and how should I do this kind of setting?
  Just pass the parameter level triggered in request_irq in your device
  driver.
 
 Hi Sign,
 
 I searched the interrupt.h for the all the defined flags that I can pass
 to the request_irq, but there is no a flag looks like level triggered.
 Would you tell me what you mean the parameter level triggered?
irq_set_irq_type(info-irq, IRQ_TYPE_LEVEL_LOW)

include/linux/irq.h
IRQ_TYPE_LEVEL_HIGH  - high level triggered
IRQ_TYPE_LEVEL_LOW   - low level triggered
 
 Thanks.
 
  
  
   Thanks in advance.
  
   --
   woody
   I can't go back to yesterday - because I was a different person then.
  
   ___
   Kernelnewbies mailing list
   kernelnewb...@kernelnewbies.org
   http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
 


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] OMAP GPIO - don't wake from suspend unless requested.

[anish kumar]

On Fri, 2012-12-14 at 18:05 +1100, NeilBrown wrote:
> On Mon, 10 Sep 2012 10:57:07 -0700 Kevin Hilman 
> wrote:
> 
> 
> > OK thanks, I'll queue this up for v3.6-rc as this should qualify as a
> > regression.
> 
> I don't think this did actually get queued.  At least I'm still seeing the
> bug in 3.7 and I cannot see a patch in the git history that looks right.
> But then I don't remember what we ended up with - it was 3 months ago!!!
> 
> This is the last thing I can find in my email history - it still seems to
> apply and still seems to work.
> 
> NeilBrown
> 
> 
> From: NeilBrown 
> Date: Mon, 10 Sep 2012 14:09:06 +1000
> Subject: [PATCH] OMAP GPIO - don't wake from suspend unless requested.
> 
> Current kernel will wake from suspend on an event an any active
> GPIO event if enable_irq_wake() wasn't called.
Should it read this way "Current kernel will wake from suspend on any
active gpio event if enable_irq_wake() wasn't called" ?
> 
> There are two reasons that the hardware wake-enable bit should be set:
> 
> 1/ while non-suspended the CPU might go into a deep sleep (off_mode)
>   in which the wake-enable bit is needed for an interrupt to be
"in which case"
>   recognised.
> 2/ while suspended the GPIO interrupt should wake from suspend if and
>only if irq_wake as been enabled.
"has been enabled"
> 
> The code currently doesn't keep these two reasons separate so they get
> confused and sometimes the wakeup flags is set incorrectly.
> 
> This patch reverts:
>  commit 9c4ed9e6c01e7a8bd9079da8267e1f03cb4761fc
> gpio/omap: remove suspend/resume callbacks
> and
>  commit 0aa2727399c0b78225021413022c164cb99fbc5e
> gpio/omap: remove suspend_wakeup field from struct gpio_bank
> 
> and makes some minor changes so that we have separate flags for "GPIO
> should wake from deep idle" and "GPIO should wake from suspend".
> 
> With this patch, the GPIO from my touch screen doesn't wake my device
> any more, which is what I want.
> 
> Cc: Kevin Hilman 
> Cc: Tony Lindgren 
> Cc: Santosh Shilimkar 
> Cc: Benoit Cousson 
> Cc: Grant Likely 
> Cc: Tarun Kanti DebBarma 
> Cc: Felipe Balbi 
> Cc: Govindraj.R 
> 
> Signed-off-by: NeilBrown 
> 
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index 4fbc208..79e1340 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -57,6 +57,7 @@ struct gpio_bank {
>   u16 irq;
>   int irq_base;
>   struct irq_domain *domain;
> + u32 suspend_wakeup;
>   u32 non_wakeup_gpios;
>   u32 enabled_non_wakeup_gpios;
>   struct gpio_regs context;
> @@ -522,11 +523,9 @@ static int _set_gpio_wakeup(struct gpio_bank *bank, int 
> gpio, int enable)
>  
>   spin_lock_irqsave(>lock, flags);
>   if (enable)
> - bank->context.wake_en |= gpio_bit;
> + bank->suspend_wakeup |= gpio_bit;
>   else
> - bank->context.wake_en &= ~gpio_bit;
> -
> - __raw_writel(bank->context.wake_en, bank->base + bank->regs->wkup_en);
> + bank->suspend_wakeup &= ~gpio_bit;
>   spin_unlock_irqrestore(>lock, flags);
>  
>   return 0;
> @@ -1157,6 +1156,49 @@ static int __devinit omap_gpio_probe(struct 
> platform_device *pdev)
>  #ifdef CONFIG_ARCH_OMAP2PLUS
>  
>  #if defined(CONFIG_PM_RUNTIME)
> +
> +#if defined(CONFIG_PM_SLEEP)
> +static int omap_gpio_suspend(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct gpio_bank *bank = platform_get_drvdata(pdev);
> + void __iomem *base = bank->base;
> + unsigned long flags;
> +
> + if (!bank->mod_usage ||
> + !bank->regs->wkup_en ||
> + !bank->context.wake_en)
> + return 0;
> +
> + spin_lock_irqsave(>lock, flags);
> + _gpio_rmw(base, bank->regs->wkup_en, 0x, 0);
> + _gpio_rmw(base, bank->regs->wkup_en, bank->suspend_wakeup, 1);
> + spin_unlock_irqrestore(>lock, flags);
> +
> + return 0;
> +}
> +
> +static int omap_gpio_resume(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct gpio_bank *bank = platform_get_drvdata(pdev);
> + void __iomem *base = bank->base;
> + unsigned long flags;
> +
> + if (!bank->mod_usage ||
> + !bank->regs->wkup_en ||
> + !bank->context.wake_en)
> + return 0;
> +
> + spin_lock_irqsave(>lock, flags);
> + _gpio_rmw(base, bank->regs->wkup_en, 0x, 0);
> + _gpio_rmw(base, bank->regs->wkup_en, bank->context.wake_en, 1);
> + spin_unlock_irqrestore(>lock, flags);
> +
> + return 0;
> +}
> +#endif /* CONFIG_PM_SLEEP */
> +
>  static void omap_gpio_restore_context(struct gpio_bank *bank);
>  
>  static int omap_gpio_runtime_suspend(struct device *dev)
> @@ -1386,11 +1428,14 @@ static void omap_gpio_restore_context(struct 
> gpio_bank *bank)
>  }
>  #endif /* CONFIG_PM_RUNTIME */
>  #else
> +#define omap_gpio_suspend NULL
> +#define omap_gpio_resume NULL
>  #define 

Re: [PATCH] OMAP GPIO - don't wake from suspend unless requested.

[anish kumar]

On Fri, 2012-12-14 at 18:05 +1100, NeilBrown wrote:
 On Mon, 10 Sep 2012 10:57:07 -0700 Kevin Hilman khil...@deeprootsystems.com
 wrote:
 
 
  OK thanks, I'll queue this up for v3.6-rc as this should qualify as a
  regression.
 
 I don't think this did actually get queued.  At least I'm still seeing the
 bug in 3.7 and I cannot see a patch in the git history that looks right.
 But then I don't remember what we ended up with - it was 3 months ago!!!
 
 This is the last thing I can find in my email history - it still seems to
 apply and still seems to work.
 
 NeilBrown
 
 
 From: NeilBrown ne...@suse.de
 Date: Mon, 10 Sep 2012 14:09:06 +1000
 Subject: [PATCH] OMAP GPIO - don't wake from suspend unless requested.
 
 Current kernel will wake from suspend on an event an any active
 GPIO event if enable_irq_wake() wasn't called.
Should it read this way Current kernel will wake from suspend on any
active gpio event if enable_irq_wake() wasn't called ?
 
 There are two reasons that the hardware wake-enable bit should be set:
 
 1/ while non-suspended the CPU might go into a deep sleep (off_mode)
   in which the wake-enable bit is needed for an interrupt to be
in which case
   recognised.
 2/ while suspended the GPIO interrupt should wake from suspend if and
only if irq_wake as been enabled.
has been enabled
 
 The code currently doesn't keep these two reasons separate so they get
 confused and sometimes the wakeup flags is set incorrectly.
 
 This patch reverts:
  commit 9c4ed9e6c01e7a8bd9079da8267e1f03cb4761fc
 gpio/omap: remove suspend/resume callbacks
 and
  commit 0aa2727399c0b78225021413022c164cb99fbc5e
 gpio/omap: remove suspend_wakeup field from struct gpio_bank
 
 and makes some minor changes so that we have separate flags for GPIO
 should wake from deep idle and GPIO should wake from suspend.
 
 With this patch, the GPIO from my touch screen doesn't wake my device
 any more, which is what I want.
 
 Cc: Kevin Hilman khil...@ti.com
 Cc: Tony Lindgren t...@atomide.com
 Cc: Santosh Shilimkar santosh.shilim...@ti.com
 Cc: Benoit Cousson b-cous...@ti.com
 Cc: Grant Likely grant.lik...@secretlab.ca
 Cc: Tarun Kanti DebBarma tarun.ka...@ti.com
 Cc: Felipe Balbi ba...@ti.com
 Cc: Govindraj.R govindraj.r...@ti.com
 
 Signed-off-by: NeilBrown ne...@suse.de
 
 diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
 index 4fbc208..79e1340 100644
 --- a/drivers/gpio/gpio-omap.c
 +++ b/drivers/gpio/gpio-omap.c
 @@ -57,6 +57,7 @@ struct gpio_bank {
   u16 irq;
   int irq_base;
   struct irq_domain *domain;
 + u32 suspend_wakeup;
   u32 non_wakeup_gpios;
   u32 enabled_non_wakeup_gpios;
   struct gpio_regs context;
 @@ -522,11 +523,9 @@ static int _set_gpio_wakeup(struct gpio_bank *bank, int 
 gpio, int enable)
  
   spin_lock_irqsave(bank-lock, flags);
   if (enable)
 - bank-context.wake_en |= gpio_bit;
 + bank-suspend_wakeup |= gpio_bit;
   else
 - bank-context.wake_en = ~gpio_bit;
 -
 - __raw_writel(bank-context.wake_en, bank-base + bank-regs-wkup_en);
 + bank-suspend_wakeup = ~gpio_bit;
   spin_unlock_irqrestore(bank-lock, flags);
  
   return 0;
 @@ -1157,6 +1156,49 @@ static int __devinit omap_gpio_probe(struct 
 platform_device *pdev)
  #ifdef CONFIG_ARCH_OMAP2PLUS
  
  #if defined(CONFIG_PM_RUNTIME)
 +
 +#if defined(CONFIG_PM_SLEEP)
 +static int omap_gpio_suspend(struct device *dev)
 +{
 + struct platform_device *pdev = to_platform_device(dev);
 + struct gpio_bank *bank = platform_get_drvdata(pdev);
 + void __iomem *base = bank-base;
 + unsigned long flags;
 +
 + if (!bank-mod_usage ||
 + !bank-regs-wkup_en ||
 + !bank-context.wake_en)
 + return 0;
 +
 + spin_lock_irqsave(bank-lock, flags);
 + _gpio_rmw(base, bank-regs-wkup_en, 0x, 0);
 + _gpio_rmw(base, bank-regs-wkup_en, bank-suspend_wakeup, 1);
 + spin_unlock_irqrestore(bank-lock, flags);
 +
 + return 0;
 +}
 +
 +static int omap_gpio_resume(struct device *dev)
 +{
 + struct platform_device *pdev = to_platform_device(dev);
 + struct gpio_bank *bank = platform_get_drvdata(pdev);
 + void __iomem *base = bank-base;
 + unsigned long flags;
 +
 + if (!bank-mod_usage ||
 + !bank-regs-wkup_en ||
 + !bank-context.wake_en)
 + return 0;
 +
 + spin_lock_irqsave(bank-lock, flags);
 + _gpio_rmw(base, bank-regs-wkup_en, 0x, 0);
 + _gpio_rmw(base, bank-regs-wkup_en, bank-context.wake_en, 1);
 + spin_unlock_irqrestore(bank-lock, flags);
 +
 + return 0;
 +}
 +#endif /* CONFIG_PM_SLEEP */
 +
  static void omap_gpio_restore_context(struct gpio_bank *bank);
  
  static int omap_gpio_runtime_suspend(struct device *dev)
 @@ -1386,11 +1428,14 @@ static void omap_gpio_restore_context(struct 
 gpio_bank *bank)
  }
  #endif /* CONFIG_PM_RUNTIME */
  #else
 +#define omap_gpio_suspend NULL
 +#define omap_gpio_resume 

Re: question about drivers/power/88pm860x_charger.c

[anish kumar]

On Sat, 2012-12-08 at 17:37 +0100, Julia Lawall wrote:
> The function pm860x_charger_probe in the file 
> drivers/power/88pm860x_charger.c contains the following code:
> 
>  count = pdev->num_resources;
>  for (i = 0, j = 0; i < count; i++) {
>  info->irq[j] = platform_get_irq(pdev, i);
>  if (info->irq[j] < 0)
>  continue;
>  j++;
>  }
>  info->irq_nums = j;
> 
> and then later the following code:
> 
> for (i = 0; i < ARRAY_SIZE(info->irq); i++) {
This seems to be wrong.We already know this size which is 7 so why
use ARRAY_SIZE?I think the intention is as below
>  ret = request_threaded_irq(info->irq[i], NULL,
>  pm860x_irq_descs[i].handler,
>  IRQF_ONESHOT, pm860x_irq_descs[i].name, info);
>   ...
>  }
> 
> and finally, in the function pm860x_charger_remove, the code:
> 
>  free_irq(info->irq[0], info);
>  for (i = 0; i < info->irq_nums; i++)
>  free_irq(info->irq[i], info);
> 
> It looks like the irq_nums field is being used to record how many 
> platform_get_irq calls were successful, but this information is not used 
> in the second block of code, where request_threaded_irq is called.  So it 
> would seem that all of the requested irqs should be freed, and not just 
> the first irq_nums of them.
> 
> The remove code also looks like a double free of info->irq[0].
> 
> Could I just get rid of the irq_nums field completely?  It doesn't seem to 
diff --git a/drivers/power/88pm860x_charger.c
b/drivers/power/88pm860x_charger.c
index 2dbeb14..821629f 100644
--- a/drivers/power/88pm860x_charger.c
+++ b/drivers/power/88pm860x_charger.c
@@ -698,7 +698,7 @@ static __devinit int pm860x_charger_probe(struct
platform_device *pdev)
 
pm860x_init_charger(info);
 
-   for (i = 0; i < ARRAY_SIZE(info->irq); i++) {
+   for (i = 0; i < info->irq_nums; i++) {
ret = request_threaded_irq(info->irq[i], NULL,
pm860x_irq_descs[i].handler,
IRQF_ONESHOT, pm860x_irq_descs[i].name, info);

> be used elsewhere.  Also, I was planning to use devm_request_threaded_irq, 
> so then there won't be a need for the explicit frees at all.

> 
> This file also contains a kfree of devm_kzalloc'd data, which is why I 
> looked at it in the first place.
> 
> thanks,
> julia
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: question about drivers/power/88pm860x_charger.c

[anish kumar]

On Sat, 2012-12-08 at 17:37 +0100, Julia Lawall wrote:
 The function pm860x_charger_probe in the file 
 drivers/power/88pm860x_charger.c contains the following code:
 
  count = pdev-num_resources;
  for (i = 0, j = 0; i  count; i++) {
  info-irq[j] = platform_get_irq(pdev, i);
  if (info-irq[j]  0)
  continue;
  j++;
  }
  info-irq_nums = j;
 
 and then later the following code:
 
 for (i = 0; i  ARRAY_SIZE(info-irq); i++) {
This seems to be wrong.We already know this size which is 7 so why
use ARRAY_SIZE?I think the intention is as below
  ret = request_threaded_irq(info-irq[i], NULL,
  pm860x_irq_descs[i].handler,
  IRQF_ONESHOT, pm860x_irq_descs[i].name, info);
   ...
  }
 
 and finally, in the function pm860x_charger_remove, the code:
 
  free_irq(info-irq[0], info);
  for (i = 0; i  info-irq_nums; i++)
  free_irq(info-irq[i], info);
 
 It looks like the irq_nums field is being used to record how many 
 platform_get_irq calls were successful, but this information is not used 
 in the second block of code, where request_threaded_irq is called.  So it 
 would seem that all of the requested irqs should be freed, and not just 
 the first irq_nums of them.
 
 The remove code also looks like a double free of info-irq[0].
 
 Could I just get rid of the irq_nums field completely?  It doesn't seem to 
diff --git a/drivers/power/88pm860x_charger.c
b/drivers/power/88pm860x_charger.c
index 2dbeb14..821629f 100644
--- a/drivers/power/88pm860x_charger.c
+++ b/drivers/power/88pm860x_charger.c
@@ -698,7 +698,7 @@ static __devinit int pm860x_charger_probe(struct
platform_device *pdev)
 
pm860x_init_charger(info);
 
-   for (i = 0; i  ARRAY_SIZE(info-irq); i++) {
+   for (i = 0; i  info-irq_nums; i++) {
ret = request_threaded_irq(info-irq[i], NULL,
pm860x_irq_descs[i].handler,
IRQF_ONESHOT, pm860x_irq_descs[i].name, info);

 be used elsewhere.  Also, I was planning to use devm_request_threaded_irq, 
 so then there won't be a need for the explicit frees at all.

 
 This file also contains a kfree of devm_kzalloc'd data, which is why I 
 looked at it in the first place.
 
 thanks,
 julia
 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] gpio: export 'debounce' attribute if supported by the gpio chip

[anish kumar]

On Fri, 2012-12-07 at 16:49 +, Alan Cox wrote:
> > I could imagine declaring the activity request buttons to be "input", but 
> > for
> > presence detects it is a bit far fetched and would add too much complexity.
> 
> Android tries to address this with its switch class driver, but I'm not
> sure its actually got anything over making them input devices.

Sorry for not understanding the context here.How the debounce sysfs
added by Guenter has anything to do with switch driver in android?

AFAIK android just uses switch driver api to talk to driver to get the
status of the headset and the status of the keys if headset has any.This
API in turn updates the sysfs to let userspace know the status change
using kobject uevent.
 
Switch driver is replaced by extcon in the mainline.
> 
> Alan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] gpio: export 'debounce' attribute if supported by the gpio chip

[anish kumar]

On Fri, 2012-12-07 at 16:49 +, Alan Cox wrote:
  I could imagine declaring the activity request buttons to be input, but 
  for
  presence detects it is a bit far fetched and would add too much complexity.
 
 Android tries to address this with its switch class driver, but I'm not
 sure its actually got anything over making them input devices.

Sorry for not understanding the context here.How the debounce sysfs
added by Guenter has anything to do with switch driver in android?

AFAIK android just uses switch driver api to talk to driver to get the
status of the headset and the status of the keys if headset has any.This
API in turn updates the sysfs to let userspace know the status change
using kobject uevent.
 
Switch driver is replaced by extcon in the mainline.
 
 Alan
 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: A typo about kernelcore= ?

[anish kumar]

On Fri, 2012-12-07 at 15:02 +0800, Han Pingtian wrote:
> Hi there,
> 
> I'm wondering this is a typo in Documentation/kernel-parameters.txt
> about "kernelcore=":
> 
>  In the event, a node is too small to have both
> kernelcore and Movable pages, kernelcore pages will
> take priority and other nodes will have a larger number
> of kernelcore pages.
> 
> I think it should be 
> 
>  In the event, a node is too small to have both
> kernelcore and Movable pages, kernelcore pages will
> take priority and other nodes will have a larger number
> of *Movable* pages.
> 
> Is it right? Thanks in advance!
adding the maintainer.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: A typo about kernelcore= ?

[anish kumar]

On Fri, 2012-12-07 at 15:02 +0800, Han Pingtian wrote:
 Hi there,
 
 I'm wondering this is a typo in Documentation/kernel-parameters.txt
 about kernelcore=:
 
  In the event, a node is too small to have both
 kernelcore and Movable pages, kernelcore pages will
 take priority and other nodes will have a larger number
 of kernelcore pages.
 
 I think it should be 
 
  In the event, a node is too small to have both
 kernelcore and Movable pages, kernelcore pages will
 take priority and other nodes will have a larger number
 of *Movable* pages.
 
 Is it right? Thanks in advance!
adding the maintainer.
 
 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] irqdomain: update documentation

[anish kumar]

On Sat, 2012-12-01 at 19:05 +0100, Linus Walleij wrote:
> This updates the IRQdomain documentation a bit, by adding a more
> verbose explanation to why we need this, and by adding some
> extended documentation of the irq_domain_simple() usecase.
> 
> Signed-off-by: Linus Walleij 
Thanks for doing this as the other day I was complaining about it 
and wanted to do this as you have already done.
> ---
>  Documentation/IRQ-domain.txt | 34 +-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/IRQ-domain.txt b/Documentation/IRQ-domain.txt
> index 1401cec..9bc9594 100644
> --- a/Documentation/IRQ-domain.txt
> +++ b/Documentation/IRQ-domain.txt
> @@ -7,6 +7,21 @@ systems with multiple interrupt controllers the kernel must 
> ensure
>  that each one gets assigned non-overlapping allocations of Linux
>  IRQ numbers.
>  
> +The number of interrupt controllers registered as unique irqchips
> +show a rising tendency: for example subdrivers of different kinds
> +such as GPIO controllers avoid reimplementing identical callback
> +mechanisms as the IRQ core system by modelling their interrupt
> +handlers as irqchips, i.e. in effect cascading interrupt controllers.
> +
> +Here the interrupt number loose all kind of correspondence to
> +hardware interrupt numbers: whereas in the past, IRQ numbers could
> +be chosen so they matched the hardware IRQ line into the root
> +interrupt controller (i.e. the component actually fireing the
> +interrupt line to the CPU) nowadays this number is just a number.
> +
> +For this reason we need a mechanism to separate controller-local
> +interrupt numbers, called hardware irq's, from Linux IRQ numbers.
> +
>  The irq_alloc_desc*() and irq_free_desc*() APIs provide allocation of
>  irq numbers, but they don't provide any support for reverse mapping of
>  the controller-local IRQ (hwirq) number into the Linux IRQ number
> @@ -40,6 +55,10 @@ required hardware setup.
>  When an interrupt is received, irq_find_mapping() function should
>  be used to find the Linux IRQ number from the hwirq number.
>  
> +The irq_create_mapping() function must be called *atleast once*
> +before any call to irq_find_mapping(), lest the descriptor will not
> +be allocated.
> +
>  If the driver has the Linux IRQ number or the irq_data pointer, and
>  needs to know the associated hwirq number (such as in the irq_chip
>  callbacks) then it can be directly obtained from irq_data->hwirq.
> @@ -119,4 +138,17 @@ numbers.
>  
>  Most users of legacy mappings should use irq_domain_add_simple() which
>  will use a legacy domain only if an IRQ range is supplied by the
> -system and will otherwise use a linear domain mapping.
> +system and will otherwise use a linear domain mapping. The semantics
> +of this call are such that if an IRQ range is specified then
> +descriptors will be allocated on-the-fly for it, and if no range is
> +specified it will fall through to irq_domain_add_linear() which meand
> +*no* irq descriptors will be allocated.
> +
> +A typical use case for simple domains is where an irqchip provider
> +is supporting both dynamic and static IRQ assignments.
> +
> +In order to avoid ending up in a situation where a linear domain is
> +used and no descriptor gets allocated it is very important to make sure
> +that the driver using the simple domain call irq_create_mapping()
> +before any irq_find_mapping() since the latter will actually work
> +for the static IRQ assignment case.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] irqdomain: update documentation

[anish kumar]

On Sat, 2012-12-01 at 19:05 +0100, Linus Walleij wrote:
 This updates the IRQdomain documentation a bit, by adding a more
 verbose explanation to why we need this, and by adding some
 extended documentation of the irq_domain_simple() usecase.
 
 Signed-off-by: Linus Walleij linus.wall...@linaro.org
Thanks for doing this as the other day I was complaining about it 
and wanted to do this as you have already done.
 ---
  Documentation/IRQ-domain.txt | 34 +-
  1 file changed, 33 insertions(+), 1 deletion(-)
 
 diff --git a/Documentation/IRQ-domain.txt b/Documentation/IRQ-domain.txt
 index 1401cec..9bc9594 100644
 --- a/Documentation/IRQ-domain.txt
 +++ b/Documentation/IRQ-domain.txt
 @@ -7,6 +7,21 @@ systems with multiple interrupt controllers the kernel must 
 ensure
  that each one gets assigned non-overlapping allocations of Linux
  IRQ numbers.
  
 +The number of interrupt controllers registered as unique irqchips
 +show a rising tendency: for example subdrivers of different kinds
 +such as GPIO controllers avoid reimplementing identical callback
 +mechanisms as the IRQ core system by modelling their interrupt
 +handlers as irqchips, i.e. in effect cascading interrupt controllers.
 +
 +Here the interrupt number loose all kind of correspondence to
 +hardware interrupt numbers: whereas in the past, IRQ numbers could
 +be chosen so they matched the hardware IRQ line into the root
 +interrupt controller (i.e. the component actually fireing the
 +interrupt line to the CPU) nowadays this number is just a number.
 +
 +For this reason we need a mechanism to separate controller-local
 +interrupt numbers, called hardware irq's, from Linux IRQ numbers.
 +
  The irq_alloc_desc*() and irq_free_desc*() APIs provide allocation of
  irq numbers, but they don't provide any support for reverse mapping of
  the controller-local IRQ (hwirq) number into the Linux IRQ number
 @@ -40,6 +55,10 @@ required hardware setup.
  When an interrupt is received, irq_find_mapping() function should
  be used to find the Linux IRQ number from the hwirq number.
  
 +The irq_create_mapping() function must be called *atleast once*
 +before any call to irq_find_mapping(), lest the descriptor will not
 +be allocated.
 +
  If the driver has the Linux IRQ number or the irq_data pointer, and
  needs to know the associated hwirq number (such as in the irq_chip
  callbacks) then it can be directly obtained from irq_data-hwirq.
 @@ -119,4 +138,17 @@ numbers.
  
  Most users of legacy mappings should use irq_domain_add_simple() which
  will use a legacy domain only if an IRQ range is supplied by the
 -system and will otherwise use a linear domain mapping.
 +system and will otherwise use a linear domain mapping. The semantics
 +of this call are such that if an IRQ range is specified then
 +descriptors will be allocated on-the-fly for it, and if no range is
 +specified it will fall through to irq_domain_add_linear() which meand
 +*no* irq descriptors will be allocated.
 +
 +A typical use case for simple domains is where an irqchip provider
 +is supporting both dynamic and static IRQ assignments.
 +
 +In order to avoid ending up in a situation where a linear domain is
 +used and no descriptor gets allocated it is very important to make sure
 +that the driver using the simple domain call irq_create_mapping()
 +before any irq_find_mapping() since the latter will actually work
 +for the static IRQ assignment case.


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] EXTCON: Get and set cable properties

[anish kumar]

On Mon, 2012-12-03 at 01:53 +, Tc, Jenny wrote:
> > We discussed about this patch and then suggest some method to resolve this
> > issue by Myungjoo Ham. Why don't you write additional feature or your
> > opinion based on following patch by Myungjoo Ham?
> > -
> > http://git.kernel.org/?p=linux/kernel/git/mzx/extcon.git;a=commitdiff;h=73
> > 12b79d69a2b9f06af4f1254bc4644751e3e3ea
> > 
> 
> I replied on the thread and pointed out issues that I see with this solution. 
> IMHO
> It's not fair to register a cable with power_supply/regulator/charger manager 
> just to
> expose the cable properties. 
Don't we do that with already in allmost all drivers?Like if I have a
keyboard driver and then I register with input framework and if the
keyboard driver needs clk services then I register with clk framework as
well and same with other services needed by keyboard driver.I think it
makes sense for a cable to register with different framework if it
supports that functionality as those services also would know that we
have a cable with so and so property.
> 
> > 
> > On 11/28/2012 06:49 AM, Jenny TC wrote:
> > > Existing EXTCON implementation doesn't give a mechanim to read the
> > > cable properties and extra states a cable needs to support. There are
> > > scenarios where a cable can have more than two
> > > states(CONNECT/DISCONNECT/SUSPEND/RESUME etc) and can have some
> > > properties associated with cables(mA)
> > >
> > > This patch introduces interface to get and set cable properties from
> > > EXTCON framework. The cable property can be set either by the extcon
> > > cable provider or by other subsystems who know the cable properties
> > > using eth API extcon_cable_set_data()
> > >
> > > When the consumer gets a notification from the extcon, it can use the
> > > extcon_cable_get_data() to get the cable properties irrespective of
> > > who provides the cable data.
> > >
> > > This gives a single interface for setting and getting the cable 
> > > properties.
> > >
> > > Signed-off-by: Jenny TC 
> > > ---
> > >  drivers/extcon/extcon-class.c |   30
> > ++
> > >  include/linux/extcon.h|   39
> > +++
> > >  2 files changed, 69 insertions(+)
> > >
> > > diff --git a/drivers/extcon/extcon-class.c
> > > b/drivers/extcon/extcon-class.c index d398821..304f343 100644
> > > --- a/drivers/extcon/extcon-class.c
> > > +++ b/drivers/extcon/extcon-class.c
> > > @@ -545,6 +545,36 @@ int extcon_unregister_notifier(struct extcon_dev
> > > *edev,  }  EXPORT_SYMBOL_GPL(extcon_unregister_notifier);
> > >
> > > +/**
> > > + * extcon_cable_set_data() - Set the data structure for a cable
> > > + * @edev:the extcon device
> > > + * @cable_index: the cable index of the correspondant
> > > + * @type:type of the data structure
> > > + * @data:
> > > + */
> > > +void extcon_cable_set_data(struct extcon_dev *edev, int cable_index,
> > > +enum extcon_cable_name type,
> > > +union extcon_cable_data data)
> > > +{
> > > + edev->cables[cable_index].type = type;
> > > + edev->cables[cable_index].data = data; }
> > > +
> > > +/**
> > > + * extcon_cable_get_data() - Get the data structure for a cable
> > > + * @edev:the extcon device
> > > + * @cable_index: the cable index of the correspondant
> > > + * @type:type of the data structure
> > > + * @data:the corresponding data structure (e.g., regulator)
> > > + */
> > > +void extcon_cable_get_data(struct extcon_dev *edev, int cable_index,
> > > +enum extcon_cable_name *type,
> > > +union extcon_cable_data *data)
> > > +{
> > > + *type = edev->cables[cable_index].type;
> > > + *data = edev->cables[cable_index].data; }
> > > +
> > >  static struct device_attribute extcon_attrs[] = {
> > >   __ATTR(state, S_IRUGO | S_IWUSR, state_show, state_store),
> > >   __ATTR_RO(name),
> > > diff --git a/include/linux/extcon.h b/include/linux/extcon.h index
> > > 2c26c14..4556cc5 100644
> > > --- a/include/linux/extcon.h
> > > +++ b/include/linux/extcon.h
> > > @@ -135,6 +135,19 @@ struct extcon_dev {
> > >   struct device_attribute *d_attrs_muex;  };
> > >
> > > +/* FIXME: Is this the right place for this structure definition?
> > > + * Do we need to move it to power_supply.h?
> > > + */
> > > +struct extcon_chrgr_cable_props {
> > > + unsigned long state;
> > > + int mA;
> > > +};
> > 
> > As I said, extcon provider driver didn't provide directly charging 
> > current(int
> > mA) and some state(unsigned long state) because the extcon provider
> > driver(Micro USB interface controller device or other device related to
> > external connector) haven't mechanism which detect dynamically charging
> > current immediately. If you wish to provide charging current data to extcon
> > consumer driver or framework, you should use regulator/power_supply
> > framework or other method in extcon provider driver.
> > 
> > The patch 

RE: [PATCH] EXTCON: Get and set cable properties

[anish kumar]

On Mon, 2012-12-03 at 01:53 +, Tc, Jenny wrote:
  We discussed about this patch and then suggest some method to resolve this
  issue by Myungjoo Ham. Why don't you write additional feature or your
  opinion based on following patch by Myungjoo Ham?
  -
  http://git.kernel.org/?p=linux/kernel/git/mzx/extcon.git;a=commitdiff;h=73
  12b79d69a2b9f06af4f1254bc4644751e3e3ea
  
 
 I replied on the thread and pointed out issues that I see with this solution. 
 IMHO
 It's not fair to register a cable with power_supply/regulator/charger manager 
 just to
 expose the cable properties. 
Don't we do that with already in allmost all drivers?Like if I have a
keyboard driver and then I register with input framework and if the
keyboard driver needs clk services then I register with clk framework as
well and same with other services needed by keyboard driver.I think it
makes sense for a cable to register with different framework if it
supports that functionality as those services also would know that we
have a cable with so and so property.
 
  
  On 11/28/2012 06:49 AM, Jenny TC wrote:
   Existing EXTCON implementation doesn't give a mechanim to read the
   cable properties and extra states a cable needs to support. There are
   scenarios where a cable can have more than two
   states(CONNECT/DISCONNECT/SUSPEND/RESUME etc) and can have some
   properties associated with cables(mA)
  
   This patch introduces interface to get and set cable properties from
   EXTCON framework. The cable property can be set either by the extcon
   cable provider or by other subsystems who know the cable properties
   using eth API extcon_cable_set_data()
  
   When the consumer gets a notification from the extcon, it can use the
   extcon_cable_get_data() to get the cable properties irrespective of
   who provides the cable data.
  
   This gives a single interface for setting and getting the cable 
   properties.
  
   Signed-off-by: Jenny TC jenny...@intel.com
   ---
drivers/extcon/extcon-class.c |   30
  ++
include/linux/extcon.h|   39
  +++
2 files changed, 69 insertions(+)
  
   diff --git a/drivers/extcon/extcon-class.c
   b/drivers/extcon/extcon-class.c index d398821..304f343 100644
   --- a/drivers/extcon/extcon-class.c
   +++ b/drivers/extcon/extcon-class.c
   @@ -545,6 +545,36 @@ int extcon_unregister_notifier(struct extcon_dev
   *edev,  }  EXPORT_SYMBOL_GPL(extcon_unregister_notifier);
  
   +/**
   + * extcon_cable_set_data() - Set the data structure for a cable
   + * @edev:the extcon device
   + * @cable_index: the cable index of the correspondant
   + * @type:type of the data structure
   + * @data:
   + */
   +void extcon_cable_set_data(struct extcon_dev *edev, int cable_index,
   +enum extcon_cable_name type,
   +union extcon_cable_data data)
   +{
   + edev-cables[cable_index].type = type;
   + edev-cables[cable_index].data = data; }
   +
   +/**
   + * extcon_cable_get_data() - Get the data structure for a cable
   + * @edev:the extcon device
   + * @cable_index: the cable index of the correspondant
   + * @type:type of the data structure
   + * @data:the corresponding data structure (e.g., regulator)
   + */
   +void extcon_cable_get_data(struct extcon_dev *edev, int cable_index,
   +enum extcon_cable_name *type,
   +union extcon_cable_data *data)
   +{
   + *type = edev-cables[cable_index].type;
   + *data = edev-cables[cable_index].data; }
   +
static struct device_attribute extcon_attrs[] = {
 __ATTR(state, S_IRUGO | S_IWUSR, state_show, state_store),
 __ATTR_RO(name),
   diff --git a/include/linux/extcon.h b/include/linux/extcon.h index
   2c26c14..4556cc5 100644
   --- a/include/linux/extcon.h
   +++ b/include/linux/extcon.h
   @@ -135,6 +135,19 @@ struct extcon_dev {
 struct device_attribute *d_attrs_muex;  };
  
   +/* FIXME: Is this the right place for this structure definition?
   + * Do we need to move it to power_supply.h?
   + */
   +struct extcon_chrgr_cable_props {
   + unsigned long state;
   + int mA;
   +};
  
  As I said, extcon provider driver didn't provide directly charging 
  current(int
  mA) and some state(unsigned long state) because the extcon provider
  driver(Micro USB interface controller device or other device related to
  external connector) haven't mechanism which detect dynamically charging
  current immediately. If you wish to provide charging current data to extcon
  consumer driver or framework, you should use regulator/power_supply
  framework or other method in extcon provider driver.
  
  The patch suggested by Myungjoo Ham define following struct:
  union extcon_cable_data {
  struct regualtor *reg;  /* EXTCON_CT_REGULATOR */
  struct power_supply *psy; /* EXTCON_CT_PSY */
  struct charger_cable *charger; /*
  

Re: Interrupt handling - Linux

[anish kumar]

On Wed, 2012-11-28 at 20:10 +0530, manty kuma wrote:
> In linux interrupt programming, we do request_irq(...) in this
> function, the first argument is irq number. If i am not wrong, this is
> the interrupt line that we are requesting from kernel. For one
Right.
>  particular hardware, is this IRQ line fixed or can it register on any
> line based on the availability? The concept is not clear. Kindly
In linux there are two concepts related to interrupt which is never
clearly mentioned anywhere(at least not that I know of) and that is why
let me clarify.
1. Hardware interrupt number.Given by your irq controller or the
hardware which is capable of generating the interrupts.
2. Software interrupt number assigned by linux interrupt handling core.

So the first question which arises in mind is: why does linux generates
the software interrupt number?Won't hardware interrupt number be enough
to keep everyone happy?

Simple reason is just for book keeping as the software interrupt numbers
generated would be linear as we are in control of what numbers to
allocate and if we start using the numbers generated by irq controller
which can generate random numbers then searching and indexing would
require expensive operations as compared to working in linear
domain(experts can add more here if I am not to the point).

However if your irq controller is capable of choosing the interrupt
numbers then linux irq number will be same as hardware interrupt number.

So let's come back to the question.So for a particular hardware the
hardware interrupt line would be always fixed as well as the software
interrupt numbers generated by the linux irq core.
 
>  explain. Also, when i do interrupt programming for AVR or ARM, all
> the peripherals are having fixed IRQ numbers. and they are having
>  handlers. There is no concept of interrupt lines as such. So my
> second question is how are IRQ lines and IRQ numbers related?
IRQ lines are connected to irq controller and you should have a look at
the driver of your irq controller as to how does it assign the hardware
irq numbers(probably by reading some registers).All the peripherals are
connected to the irq controller such as keyboard and mouse and they have
fixed irq lines.Once a signal is asserted the irq controller raises an
interrupt to arm core and arm core in turn raises a hardware
interrupt.This hardware interrupt will call into linux irq handling
code.Which interrupt handler to be called is already decided by the
individual drivers, remember they have called request_irq with an
interrupt number.
This interrupt number would be a software interrupt number as explained
before and this number to hardware interrupt number association is done
by the interrupt controller or the chip driver which is capable of
taking(handling) one interrupt and calling individual interrupt handlers
after reading the corresponding registers(read handle_nested_irq).
This conversion of hardware interrupt number to software interrupt
number is done in /kernel/irq/irqdomain.c file.
 
PS:I may be wrong but this description is from what I have read in the
code.Please do point out any mistakes.
> 
> 
> Thanks,
> Sandeep
> ___
> Kernelnewbies mailing list
> kernelnewb...@kernelnewbies.org
> http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Interrupt handling - Linux

[anish kumar]

On Wed, 2012-11-28 at 20:10 +0530, manty kuma wrote:
 In linux interrupt programming, we do request_irq(...) in this
 function, the first argument is irq number. If i am not wrong, this is
 the interrupt line that we are requesting from kernel. For one
Right.
  particular hardware, is this IRQ line fixed or can it register on any
 line based on the availability? The concept is not clear. Kindly
In linux there are two concepts related to interrupt which is never
clearly mentioned anywhere(at least not that I know of) and that is why
let me clarify.
1. Hardware interrupt number.Given by your irq controller or the
hardware which is capable of generating the interrupts.
2. Software interrupt number assigned by linux interrupt handling core.

So the first question which arises in mind is: why does linux generates
the software interrupt number?Won't hardware interrupt number be enough
to keep everyone happy?

Simple reason is just for book keeping as the software interrupt numbers
generated would be linear as we are in control of what numbers to
allocate and if we start using the numbers generated by irq controller
which can generate random numbers then searching and indexing would
require expensive operations as compared to working in linear
domain(experts can add more here if I am not to the point).

However if your irq controller is capable of choosing the interrupt
numbers then linux irq number will be same as hardware interrupt number.

So let's come back to the question.So for a particular hardware the
hardware interrupt line would be always fixed as well as the software
interrupt numbers generated by the linux irq core.
 
  explain. Also, when i do interrupt programming for AVR or ARM, all
 the peripherals are having fixed IRQ numbers. and they are having
  handlers. There is no concept of interrupt lines as such. So my
 second question is how are IRQ lines and IRQ numbers related?
IRQ lines are connected to irq controller and you should have a look at
the driver of your irq controller as to how does it assign the hardware
irq numbers(probably by reading some registers).All the peripherals are
connected to the irq controller such as keyboard and mouse and they have
fixed irq lines.Once a signal is asserted the irq controller raises an
interrupt to arm core and arm core in turn raises a hardware
interrupt.This hardware interrupt will call into linux irq handling
code.Which interrupt handler to be called is already decided by the
individual drivers, remember they have called request_irq with an
interrupt number.
This interrupt number would be a software interrupt number as explained
before and this number to hardware interrupt number association is done
by the interrupt controller or the chip driver which is capable of
taking(handling) one interrupt and calling individual interrupt handlers
after reading the corresponding registers(read handle_nested_irq).
This conversion of hardware interrupt number to software interrupt
number is done in /kernel/irq/irqdomain.c file.
 
PS:I may be wrong but this description is from what I have read in the
code.Please do point out any mistakes.
 
 
 Thanks,
 Sandeep
 ___
 Kernelnewbies mailing list
 kernelnewb...@kernelnewbies.org
 http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH] extcon : callback function to read cable property

[anish kumar]

On Fri, 2012-11-09 at 14:05 +, Tc, Jenny wrote:
> > I think that the role of extcon subsystem notify changed
> > state(attached/detached) of cable to notifiee, but if you want to add
> > property feature of cable, you should solve ambiguous issues.
> > 
> > First,
> > This patch only support the properties of charger cable but, never support
> > property of other cable. The following structure depend on only charger
> > cable. We can check it the following structure:
> > struct extcon_chrg_cbl_props {
> > enum extcon_chrgr_cbl_stat cable_state;
> > unsigned long mA;
> > };
> > 
> > I think that the patch have to support all of cables for property feature.
> 
> My suggestion is to have a structure like this 
> 
> struct  extcon_cablel_props {
>   unsigned int cable_state;
>   unsigned int data; 
Why can't it be float/long/double??
> }
> Not all cables will have more than two states. If any cable has more than two 
> states,
> the above structure makes it flexible to represent additional state and the 
> data associated
> 
> > 
> > Second,
> > Certainly, you should define common properties of all cables and specific
> > properties of each cable. The properties of charger cable should never be
> > defined only.
IMHO the extcon doesn't know anything about the cable except the state
which is currently it is in and which also is set by the provider.I am
unable to understand why should extcon provide more than what it
knows?It should just limit itself to broadcasting the cable state and
exploiting it for any other information would only lead to more
un-necessary code.
It should be same as IIO subsystem where the consumer and provider knows
before hand what information they are going to share and with what
precision and IIO core is just a way to do that.It doesn't know anything
beyond what is given by the provider.
Same is the case with driver core where individual driver sets it's own
private data and the driver core just gives that private data back to
the driver as and when it needs but parsing the private data in the
right way is up to the individual driver.

I fail to understand why is not the case here. 
> 
> Hope above structure would be enough to represent the common cable state and
> it's data. If a cable has more than two states, then extcon_update_state can 
> be used to
> notify the consumer
> 1)Provider can just toggle the "state" argument to notify the consumer that 
> cable state is 
> changed
> OR
> 2) Add one more argument  like  extcon_update_state(struct extcon_dev *edev, 
> u32 mask, u32 state1, u32 sate2)
This will cause other drivers to change such as arizona.
> If the state2 is set, then consumers can use get_cable_properties() to query 
> the cable properties. State2 need to be
> used only if the cable need to represent more than two state
> 
> > 
> > Third,
> > If we finish to decide the properties for all cables, I'd like to see a 
> > example
Why do we think that state and property is the only thing which the
consumer want to know?I am sure there would be some more properties
which would be of some interest to consumers and there is quite a
possibility that in future we might get a patch for that also.So instead
of that limiting it to just state is a good idea.
> > code.
> 
> Agreed. If we  agree on the above structure, I can modify the charging 
> subsystem patch
> to use the same structure. (https://lkml.org/lkml/2012/10/18/219). This would 
> give a reference
> for the new feature.
> 
> > 
> > You explained following changed state of USB according to Host state on
> > other patch.
> > ---
> > For USB2.0
> > 1) CONNECT (100mA)->UPDATE(500mA)->DISCONNECT(0mA)
> > 2) CONNECT (100mA)->UPDATE(500mA)->HOST SUSPEND(2.5mA/500mA)-
> > >DISCONNECT(0mA)
> > 3) CONNECT (100mA)->UPDATE(500mA)->HOST SUSPEND(2.5mA/500mA)-
> > >HOST RESUME(500mA)->DISCONNECT(0mA)
> > 
> > For USB 3.0
> > 4) CONNECT (150mA)->UPDATE(900mA)->DISCONNECT(0mA)
> > 5) CONNECT (150mA)->UPDATE(900mA)-> HOST SUSPEND(2.5mA/900mA)-
> > >DISCONNECT(0mA)
> > 6) CONNECT (100mA)->UPDATE(900mA)->HOST SUSPEND(2.5mA/900mA)-
> > >HOST RESUME(900mA)->DISCONNECT(0mA)
> > ---
> > 
> > I have a question. Can the provider device driver(e.g., extcon-max77693.c/
> > extcon-max8997.c) detect the changed state of host? I'd like to see the
> > example device driver that the provider device driver detect changed state
> > of host.
> > Could you provide example device driver?
> 
> Good question. The OTG drivers are capable of identifying the SUSPEND event.
> System cannot setup SDP (USB host) charging with maximum charging current - 
> 500mA
> (USB2.0/ 900mA(USB 3)) without enumerating the USB. The USB enumeration can be
> done only with a USB/OTG driver. IMHO the above extcon drivers
> (extcon-max77693.c/ extcon-max8997.c) are not capable of doing the USB 
> enumeration
> and identifying the charge current. They can just identify the charger type - 
> 

RE: [PATCH] extcon : callback function to read cable property

[anish kumar]

On Fri, 2012-11-09 at 14:05 +, Tc, Jenny wrote:
  I think that the role of extcon subsystem notify changed
  state(attached/detached) of cable to notifiee, but if you want to add
  property feature of cable, you should solve ambiguous issues.
  
  First,
  This patch only support the properties of charger cable but, never support
  property of other cable. The following structure depend on only charger
  cable. We can check it the following structure:
  struct extcon_chrg_cbl_props {
  enum extcon_chrgr_cbl_stat cable_state;
  unsigned long mA;
  };
  
  I think that the patch have to support all of cables for property feature.
 
 My suggestion is to have a structure like this 
 
 struct  extcon_cablel_props {
   unsigned int cable_state;
   unsigned int data; 
Why can't it be float/long/double??
 }
 Not all cables will have more than two states. If any cable has more than two 
 states,
 the above structure makes it flexible to represent additional state and the 
 data associated
 
  
  Second,
  Certainly, you should define common properties of all cables and specific
  properties of each cable. The properties of charger cable should never be
  defined only.
IMHO the extcon doesn't know anything about the cable except the state
which is currently it is in and which also is set by the provider.I am
unable to understand why should extcon provide more than what it
knows?It should just limit itself to broadcasting the cable state and
exploiting it for any other information would only lead to more
un-necessary code.
It should be same as IIO subsystem where the consumer and provider knows
before hand what information they are going to share and with what
precision and IIO core is just a way to do that.It doesn't know anything
beyond what is given by the provider.
Same is the case with driver core where individual driver sets it's own
private data and the driver core just gives that private data back to
the driver as and when it needs but parsing the private data in the
right way is up to the individual driver.

I fail to understand why is not the case here. 
 
 Hope above structure would be enough to represent the common cable state and
 it's data. If a cable has more than two states, then extcon_update_state can 
 be used to
 notify the consumer
 1)Provider can just toggle the state argument to notify the consumer that 
 cable state is 
 changed
 OR
 2) Add one more argument  like  extcon_update_state(struct extcon_dev *edev, 
 u32 mask, u32 state1, u32 sate2)
This will cause other drivers to change such as arizona.
 If the state2 is set, then consumers can use get_cable_properties() to query 
 the cable properties. State2 need to be
 used only if the cable need to represent more than two state
 
  
  Third,
  If we finish to decide the properties for all cables, I'd like to see a 
  example
Why do we think that state and property is the only thing which the
consumer want to know?I am sure there would be some more properties
which would be of some interest to consumers and there is quite a
possibility that in future we might get a patch for that also.So instead
of that limiting it to just state is a good idea.
  code.
 
 Agreed. If we  agree on the above structure, I can modify the charging 
 subsystem patch
 to use the same structure. (https://lkml.org/lkml/2012/10/18/219). This would 
 give a reference
 for the new feature.
 
  
  You explained following changed state of USB according to Host state on
  other patch.
  ---
  For USB2.0
  1) CONNECT (100mA)-UPDATE(500mA)-DISCONNECT(0mA)
  2) CONNECT (100mA)-UPDATE(500mA)-HOST SUSPEND(2.5mA/500mA)-
  DISCONNECT(0mA)
  3) CONNECT (100mA)-UPDATE(500mA)-HOST SUSPEND(2.5mA/500mA)-
  HOST RESUME(500mA)-DISCONNECT(0mA)
  
  For USB 3.0
  4) CONNECT (150mA)-UPDATE(900mA)-DISCONNECT(0mA)
  5) CONNECT (150mA)-UPDATE(900mA)- HOST SUSPEND(2.5mA/900mA)-
  DISCONNECT(0mA)
  6) CONNECT (100mA)-UPDATE(900mA)-HOST SUSPEND(2.5mA/900mA)-
  HOST RESUME(900mA)-DISCONNECT(0mA)
  ---
  
  I have a question. Can the provider device driver(e.g., extcon-max77693.c/
  extcon-max8997.c) detect the changed state of host? I'd like to see the
  example device driver that the provider device driver detect changed state
  of host.
  Could you provide example device driver?
 
 Good question. The OTG drivers are capable of identifying the SUSPEND event.
 System cannot setup SDP (USB host) charging with maximum charging current - 
 500mA
 (USB2.0/ 900mA(USB 3)) without enumerating the USB. The USB enumeration can be
 done only with a USB/OTG driver. IMHO the above extcon drivers
 (extcon-max77693.c/ extcon-max8997.c) are not capable of doing the USB 
 enumeration
 and identifying the charge current. They can just identify the charger type - 
 SDP/DCP/CDP/ACA/AC. The intelligence for USB enumeration 
 should be inside USB/OTG driver.
 


--
To unsubscribe from this list: send the line unsubscribe 

Re: How does atomic operation work on smp

[anish kumar]

On Wed, 2012-11-07 at 17:45 +0100, Nicholas Mc Guire wrote:
> On Wed, 07 Nov 2012, Hendrik Visage wrote:
> 
> > On Thu, Nov 8, 2012 at 9:01 AM, Jimmy Pan  wrote:
> > > I understand how atomic operation work on unary core processors, I think 
> > > it just disables the interrupt and dominate the cpu until it finished.
> 
> Atomic operations do not need to disable interrupts - a test-and-set 
> operation does not disable interrupts. Locking primitives like spin-locks 
> that allow you to get consistent access to non-atomic critical sections may 
> disable interrupts.
> 
> It seems to me that you are identifying atomic operations with critical 
> sections, which is not really the same thing - but maybe I missunderstood you.
> 
> > > While, how do we implement this on multi processor computers?
> > > Suppose cpu A is performing an atomic operation on variable a. At the 
> > > same time, cpu B is also performing the operation on a. In such the 
> > > result may be overwritten.
> > > Of course we can use spinlocks, but on the atomic operation's behalf, how 
> > > does it gurantee to prevent such case?
> > > Can anyone explain the crux of it? Thanks.
> > 
> > Basically you make use of machine specific instructions that will  do
> > that for you.
> > In other words, get the datasheets of the specific system you intent
> > to code on/for (I assume here you are refering to assembler level
> > codes, as higher up you make use of the relevant libraries that does
> > that for you).
> >
> 
> More or less any (sane) architecture will provide some very basic atoicity
Architecture provides minimum atomicity in the form of assembly
instructions.So below is the code of how in arm atomic_add() is
implemented which is SMP safe,
/*
 * ARMv6 UP and SMP safe atomic ops.  We use load exclusive and
 * store exclusive to ensure that these are atomic.  We may loop
 * to ensure that the update happens.
 */
static inline void atomic_add(int i, atomic_t *v)
{
 __asm__ __volatile__("@ atomic_add\n"
"1: ldrex   %0, [%3]\n"
"   add %0, %0, %4\n"
"   strex   %1, %0, [%3]\n"
"   teq %1, #0\n"
"   bne 1b"
: "=" (result), "=" (tmp), "+Qo" (v->counter)
: "r" (>counter), "Ir" (i)
: "cc");
}

Above function is SMP safe(but sometimes this also needs memory barrier
(/Documentation/memory-barriers.txt-ATOMIC OPERATIONS) and be careful
with the name of the functions as atomic in the function name doesn't
guaranty anything as below:
CPU 1CPU 2
===   ===
x = 0;   y = x;
   atomic_add_return(x)  

what do you think would be the value of y in CPU2?Legally it can have
any value i.e. 0 or 1 as CPU1 can reorder the instructions any time(when
does it see fit to reorder?).However if atomic_add_return is implemented
with explicit memory barriers then it wouldn't happen and you will see a
correct result as below:
CPU 1CPU 2
===   ===
x = 0;   
   atomic_add_return(x)  
y  = x; //x will be 1 here

>From arch/arm/include/asm/atomic.h 
static inline int atomic_add_return(int i, atomic_t *v)
{
smp_mb();  
__asm__ __volatile__("@ atomic_add_return\n"
"1: ldrex   %0, [%3]\n"
"   add %0, %0, %4\n"
"   strex   %1, %0, [%3]\n"
"   teq %1, #0\n"
"   bne 1b"
: "=" (result), "=" (tmp), "+Qo" (v->counter)
: "r" (>counter), "Ir" (i)
: "cc");

smp_mb(); 
}
so atomic variable is just like any other variable except it comes with
some set of operations which combined with barriers make it really
_atomic_.
> capabilities. a 32bit (or 64bit on 64bit boxes) data object is guaranteed
> to be consistent (that is you will never be able to get half of the old and
> half of the new value if there were a concurrent read and write). Some form
> of Compare and Swap (CAS) / Test and Set bit assembler instructuion will be
> provided that can overcome the lowest level race that would be incured by
> setting and then testing in separate steps. And finally low level
> mechanisms are provided to ensure consistency over cores by load/store
> fences (memory barriers). 
> 
> In the above case where you have a concurrent reader and writer of variable
> "a" you do not need any lock provided the data object is only a single 32/64
> bit object - if it is say a struct then you need locking to ensure consistency
> of the retrieved data. The atomic instructions are then actually only needed
> for the locks (but note that locks here are pure advisory locks not enforced
> in any way by HW). 
> 
> so if you have a writer updating a variable and a reader reading it 
> concurently
> then the reader is guaranteed to always read a consistent 32bit (or 64bit) 
> entity - but it is not guaranteed of course that you actually read 

Re: How does atomic operation work on smp

[anish kumar]

On Wed, 2012-11-07 at 17:45 +0100, Nicholas Mc Guire wrote:
 On Wed, 07 Nov 2012, Hendrik Visage wrote:
 
  On Thu, Nov 8, 2012 at 9:01 AM, Jimmy Pan dsp...@gmail.com wrote:
   I understand how atomic operation work on unary core processors, I think 
   it just disables the interrupt and dominate the cpu until it finished.
 
 Atomic operations do not need to disable interrupts - a test-and-set 
 operation does not disable interrupts. Locking primitives like spin-locks 
 that allow you to get consistent access to non-atomic critical sections may 
 disable interrupts.
 
 It seems to me that you are identifying atomic operations with critical 
 sections, which is not really the same thing - but maybe I missunderstood you.
 
   While, how do we implement this on multi processor computers?
   Suppose cpu A is performing an atomic operation on variable a. At the 
   same time, cpu B is also performing the operation on a. In such the 
   result may be overwritten.
   Of course we can use spinlocks, but on the atomic operation's behalf, how 
   does it gurantee to prevent such case?
   Can anyone explain the crux of it? Thanks.
  
  Basically you make use of machine specific instructions that will  do
  that for you.
  In other words, get the datasheets of the specific system you intent
  to code on/for (I assume here you are refering to assembler level
  codes, as higher up you make use of the relevant libraries that does
  that for you).
 
 
 More or less any (sane) architecture will provide some very basic atoicity
Architecture provides minimum atomicity in the form of assembly
instructions.So below is the code of how in arm atomic_add() is
implemented which is SMP safe,
/*
 * ARMv6 UP and SMP safe atomic ops.  We use load exclusive and
 * store exclusive to ensure that these are atomic.  We may loop
 * to ensure that the update happens.
 */
static inline void atomic_add(int i, atomic_t *v)
{
 __asm__ __volatile__(@ atomic_add\n
1: ldrex   %0, [%3]\n
   add %0, %0, %4\n
   strex   %1, %0, [%3]\n
   teq %1, #0\n
   bne 1b
: =r (result), =r (tmp), +Qo (v-counter)
: r (v-counter), Ir (i)
: cc);
}

Above function is SMP safe(but sometimes this also needs memory barrier
(/Documentation/memory-barriers.txt-ATOMIC OPERATIONS) and be careful
with the name of the functions as atomic in the function name doesn't
guaranty anything as below:
CPU 1CPU 2
===   ===
x = 0;   y = x;
   atomic_add_return(x)  

what do you think would be the value of y in CPU2?Legally it can have
any value i.e. 0 or 1 as CPU1 can reorder the instructions any time(when
does it see fit to reorder?).However if atomic_add_return is implemented
with explicit memory barriers then it wouldn't happen and you will see a
correct result as below:
CPU 1CPU 2
===   ===
x = 0;   
   atomic_add_return(x)  
y  = x; //x will be 1 here

From arch/arm/include/asm/atomic.h 
static inline int atomic_add_return(int i, atomic_t *v)
{
smp_mb();  BARRIER HERE
__asm__ __volatile__(@ atomic_add_return\n
1: ldrex   %0, [%3]\n
   add %0, %0, %4\n
   strex   %1, %0, [%3]\n
   teq %1, #0\n
   bne 1b
: =r (result), =r (tmp), +Qo (v-counter)
: r (v-counter), Ir (i)
: cc);

smp_mb(); BARRIER HERE
}
so atomic variable is just like any other variable except it comes with
some set of operations which combined with barriers make it really
_atomic_.
 capabilities. a 32bit (or 64bit on 64bit boxes) data object is guaranteed
 to be consistent (that is you will never be able to get half of the old and
 half of the new value if there were a concurrent read and write). Some form
 of Compare and Swap (CAS) / Test and Set bit assembler instructuion will be
 provided that can overcome the lowest level race that would be incured by
 setting and then testing in separate steps. And finally low level
 mechanisms are provided to ensure consistency over cores by load/store
 fences (memory barriers). 
 
 In the above case where you have a concurrent reader and writer of variable
 a you do not need any lock provided the data object is only a single 32/64
 bit object - if it is say a struct then you need locking to ensure consistency
 of the retrieved data. The atomic instructions are then actually only needed
 for the locks (but note that locks here are pure advisory locks not enforced
 in any way by HW). 
 
 so if you have a writer updating a variable and a reader reading it 
 concurently
 then the reader is guaranteed to always read a consistent 32bit (or 64bit) 
 entity - but it is not guaranteed of course that you actually read all 
 intermediate values (that is completness is not guaranteed). If the variable
 

[Q][Process Step Wise]

[anish kumar]

Hello,

I have below question and I would really appreciate a _CORRECT_ answer.

What are the sequence of steps that happen in CPU, cache, TLB, VM, HDD
leading to execution of “x = 7” which isn’t present in cache or sysmem
nor translation in TLB. Also specify if any interrupts, exceptions or
faults are generated.

I have given a rough idea here. Not very clear about specific interrupts
at each step.I request to please correct me and provide a precise
answer.

1) CPU first fetches the instruction x = 7 from the Instruction cache
(when it reads this address in the PC/IR)
2) After decoding and executing the instruction, it sees that, it needs
to access the memory location of variable x (which will be a virtual
address)
3) Hence it issues a request to the TLB to return the physical
address/tag. 
Assuming the cache is Virtually indexed, it will parallely calculate the
index for this virtual address.
4) Since it's a TLB miss, it accesses the Page table which resides
mostly in Memory.Page table can also reside in Hardware as well in that
case it will page traversal.
//Not sure what the interrupt here is?
5) But since, the translation is not found, meaning the page for this
address is not in RAM, it issues a DMA request to transfer the page from
Secondary storage to the RAM. It knows the address of the page on
Secondary storage through the vm_area struct for this process, which
maintains the location of all the pages. 
// This is done in page fault handler. Page fault is raised when this
even occurs.
6) Once DMA is complete, the processor is interrupted with this event.
It then updates the page table with this entry and also the TLB.
//This would be an I/O interrupt to the processor
7) Once it gets the tag, it checks if that tag matches in the cache. 
8) It won't, since cache does not have this entry. 
9) Hence it fetches this block (cache block) from memory and places into
the cache and restarts the execution. 
10) In the MEM phase of the execution pipeline, it writes the value 7 to
this location in the cache.

As I understand the exact steps which happens in the processor is
specific to the arch but what I want is the standard way in which linux
deals with different archs.

As always thanks to Linux and the community.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [patch] power: battery: pointer math issue in gab_probe()

[anish kumar]

Hello Dan,

Is this patch of yours picked up by anyone?
On Sun, 2012-09-30 at 09:38 +0530, anish kumar wrote:
> On Sat, 2012-09-29 at 10:13 +0300, Dan Carpenter wrote:
> > psy->properties is an enum (32 bit type) so adding sizeof() puts us
> > four times further along than we intended.  It should be cast to a char
> > pointer before doing the math.
> You really read this driver to find out this one.
> Good one.
> > 
> > Signed-off-by: Dan Carpenter 
> > ---
> > Casting to void * would also work on GCC, at least.
> > 
> > diff --git a/drivers/power/generic-adc-battery.c 
> > b/drivers/power/generic-adc-battery.c
> > index 9bdf444..776f118 100644
> > --- a/drivers/power/generic-adc-battery.c
> > +++ b/drivers/power/generic-adc-battery.c
> > @@ -279,7 +279,8 @@ static int __devinit gab_probe(struct platform_device 
> > *pdev)
> > }
> >  
> > memcpy(psy->properties, gab_props, sizeof(gab_props));
> > -   properties = psy->properties + sizeof(gab_props);
> > +   properties = (enum power_supply_property *)
> > +   ((char *)psy->properties + sizeof(gab_props));
> >  
> > /*
> >  * getting channel from iio and copying the battery properties
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [patch] power: battery: pointer math issue in gab_probe()

[anish kumar]

Hello Dan,

Is this patch of yours picked up by anyone?
On Sun, 2012-09-30 at 09:38 +0530, anish kumar wrote:
 On Sat, 2012-09-29 at 10:13 +0300, Dan Carpenter wrote:
  psy-properties is an enum (32 bit type) so adding sizeof() puts us
  four times further along than we intended.  It should be cast to a char
  pointer before doing the math.
 You really read this driver to find out this one.
 Good one.
  
  Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
  ---
  Casting to void * would also work on GCC, at least.
  
  diff --git a/drivers/power/generic-adc-battery.c 
  b/drivers/power/generic-adc-battery.c
  index 9bdf444..776f118 100644
  --- a/drivers/power/generic-adc-battery.c
  +++ b/drivers/power/generic-adc-battery.c
  @@ -279,7 +279,8 @@ static int __devinit gab_probe(struct platform_device 
  *pdev)
  }
   
  memcpy(psy-properties, gab_props, sizeof(gab_props));
  -   properties = psy-properties + sizeof(gab_props);
  +   properties = (enum power_supply_property *)
  +   ((char *)psy-properties + sizeof(gab_props));
   
  /*
   * getting channel from iio and copying the battery properties
 


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Q][Process Step Wise]

[anish kumar]

Hello,

I have below question and I would really appreciate a _CORRECT_ answer.

What are the sequence of steps that happen in CPU, cache, TLB, VM, HDD
leading to execution of “x = 7” which isn’t present in cache or sysmem
nor translation in TLB. Also specify if any interrupts, exceptions or
faults are generated.

I have given a rough idea here. Not very clear about specific interrupts
at each step.I request to please correct me and provide a precise
answer.

1) CPU first fetches the instruction x = 7 from the Instruction cache
(when it reads this address in the PC/IR)
2) After decoding and executing the instruction, it sees that, it needs
to access the memory location of variable x (which will be a virtual
address)
3) Hence it issues a request to the TLB to return the physical
address/tag. 
Assuming the cache is Virtually indexed, it will parallely calculate the
index for this virtual address.
4) Since it's a TLB miss, it accesses the Page table which resides
mostly in Memory.Page table can also reside in Hardware as well in that
case it will page traversal.
//Not sure what the interrupt here is?
5) But since, the translation is not found, meaning the page for this
address is not in RAM, it issues a DMA request to transfer the page from
Secondary storage to the RAM. It knows the address of the page on
Secondary storage through the vm_area struct for this process, which
maintains the location of all the pages. 
// This is done in page fault handler. Page fault is raised when this
even occurs.
6) Once DMA is complete, the processor is interrupted with this event.
It then updates the page table with this entry and also the TLB.
//This would be an I/O interrupt to the processor
7) Once it gets the tag, it checks if that tag matches in the cache. 
8) It won't, since cache does not have this entry. 
9) Hence it fetches this block (cache block) from memory and places into
the cache and restarts the execution. 
10) In the MEM phase of the execution pipeline, it writes the value 7 to
this location in the cache.

As I understand the exact steps which happens in the processor is
specific to the arch but what I want is the standard way in which linux
deals with different archs.

As always thanks to Linux and the community.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] IRQ CORE: irq_work_queue function return value not used.

[anish kumar]

From: anish kumar 

As no one is using the return value of irq_work_queue function
it is better to just make it void.

Acked-by: Steven Rostedt 
Signed-off-by: anish kumar 
---
 kernel/irq_work.c |5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/kernel/irq_work.c b/kernel/irq_work.c
index 1588e3b..4829a31 100644
--- a/kernel/irq_work.c
+++ b/kernel/irq_work.c
@@ -79,17 +79,16 @@ static void __irq_work_queue(struct irq_work *work)
  *
  * Can be re-enqueued while the callback is still in progress.
  */
-bool irq_work_queue(struct irq_work *work)
+void irq_work_queue(struct irq_work *work)
 {
if (!irq_work_claim(work)) {
/*
 * Already enqueued, can't do!
 */
-   return false;
+   return;
}
 
__irq_work_queue(work);
-   return true;
 }
 EXPORT_SYMBOL_GPL(irq_work_queue);
 
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] IRQ CORE: irq_work_queue function return value not used.

[anish kumar]

From: anish kumar anish198519851...@gmail.com

As no one is using the return value of irq_work_queue function
it is better to just make it void.

Acked-by: Steven Rostedt rost...@goodmis.org
Signed-off-by: anish kumar anish198519851...@gmail.com
---
 kernel/irq_work.c |5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/kernel/irq_work.c b/kernel/irq_work.c
index 1588e3b..4829a31 100644
--- a/kernel/irq_work.c
+++ b/kernel/irq_work.c
@@ -79,17 +79,16 @@ static void __irq_work_queue(struct irq_work *work)
  *
  * Can be re-enqueued while the callback is still in progress.
  */
-bool irq_work_queue(struct irq_work *work)
+void irq_work_queue(struct irq_work *work)
 {
if (!irq_work_claim(work)) {
/*
 * Already enqueued, can't do!
 */
-   return false;
+   return;
}
 
__irq_work_queue(work);
-   return true;
 }
 EXPORT_SYMBOL_GPL(irq_work_queue);
 
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] IRQ CORE: irq_work_queue function return value not used.

[anish kumar]

From: anish kumar 

As no one is using the return value of irq_work_queue function
it is better to just make it void.
Signed-off-by: anish kumar 
---
 kernel/irq_work.c |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/irq_work.c b/kernel/irq_work.c
index 1588e3b..a85c4b3 100644
--- a/kernel/irq_work.c
+++ b/kernel/irq_work.c
@@ -79,17 +79,17 @@ static void __irq_work_queue(struct irq_work *work)
  *
  * Can be re-enqueued while the callback is still in progress.
  */
-bool irq_work_queue(struct irq_work *work)
+void irq_work_queue(struct irq_work *work)
 {
if (!irq_work_claim(work)) {
/*
 * Already enqueued, can't do!
 */
-   return false;
+   return;
}
 
__irq_work_queue(work);
-   return true;
+   return;
 }
 EXPORT_SYMBOL_GPL(irq_work_queue);
 
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] IRQ CORE: irq_work_queue function return value not used.

[anish kumar]

From: anish kumar anish198519851...@gmail.com

As no one is using the return value of irq_work_queue function
it is better to just make it void.
Signed-off-by: anish kumar anish198519851...@gmail.com
---
 kernel/irq_work.c |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/irq_work.c b/kernel/irq_work.c
index 1588e3b..a85c4b3 100644
--- a/kernel/irq_work.c
+++ b/kernel/irq_work.c
@@ -79,17 +79,17 @@ static void __irq_work_queue(struct irq_work *work)
  *
  * Can be re-enqueued while the callback is still in progress.
  */
-bool irq_work_queue(struct irq_work *work)
+void irq_work_queue(struct irq_work *work)
 {
if (!irq_work_claim(work)) {
/*
 * Already enqueued, can't do!
 */
-   return false;
+   return;
}
 
__irq_work_queue(work);
-   return true;
+   return;
 }
 EXPORT_SYMBOL_GPL(irq_work_queue);
 
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [RFC]IRQ CORE: irq_work_queue function return value not used.

[anish kumar]

On Wed, 2012-10-31 at 10:15 -0400, Steven Rostedt wrote:
> On Wed, 2012-10-31 at 23:02 +0900, anish kumar wrote:
> > From: anish kumar 
> > 
> > As no one is using the return value of irq_work_queue function
> > it is better to just make it void.
> > 
> > This patch is just a way to understand if there is some future
> > plan to use it but in any case please let me know the reason.
> > ---
> >  kernel/irq_work.c |   21 ++---
> >  1 files changed, 10 insertions(+), 11 deletions(-)
> > 
> > diff --git a/kernel/irq_work.c b/kernel/irq_work.c
> > index 1588e3b..4a9a44c 100644
> > --- a/kernel/irq_work.c
> > +++ b/kernel/irq_work.c
> > @@ -32,21 +32,21 @@ static DEFINE_PER_CPU(struct llist_head, irq_work_list);
> >  /*
> >   * Claim the entry so that no one else will poke at it.
> >   */
> > -static bool irq_work_claim(struct irq_work *work)
> > +static void irq_work_claim(struct irq_work *work)
> >  {
> > unsigned long flags, nflags;
> >  
> > for (;;) {
> > flags = work->flags;
> > if (flags & IRQ_WORK_PENDING)
> > -   return false;
> > +   return;
> > nflags = flags | IRQ_WORK_FLAGS;
> > if (cmpxchg(>flags, flags, nflags) == flags)
> > break;
> > cpu_relax();
> > }
> >  
> > -   return true;
> > +   return;
> >  }
> >  
> >  void __weak arch_irq_work_raise(void)
> > @@ -79,15 +79,14 @@ static void __irq_work_queue(struct irq_work *work)
> >   *
> >   * Can be re-enqueued while the callback is still in progress.
> >   */
> > -bool irq_work_queue(struct irq_work *work)
> > +void irq_work_queue(struct irq_work *work)
> >  {
> > -   if (!irq_work_claim(work)) {
> > -   /*
> > -* Already enqueued, can't do!
> > -*/
> > -   return false;
> > -   }
> > -
> > +   /*
> > +* This function either will claim the entry to queue
> > +* the work or if the work is already queued and is in
> > +* pending state then it will simply return.
> > +*/
> > +   irq_work_claim(work)
> 
> Um, no.
> 
> If the state was already pending, we will corrupt the llist node of the
> work if we call irq_work_queue(). You must check the return value of
> irq_work_claim() and return if it fails. You can not call
> __irq_work_queue() if irq_work_claim() does not succeed.
> 
> The return value of irq_work_queue() can be ignored, but not
> irq_work_claim().
Oh I didn't see that logic properly and rightly pointed out by you, we
should _just_ return instead of queuing the work if the state was
already pending.
> 
> -- Steve
> 
> > __irq_work_queue(work);
> > return true;
> >  }
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [RFC]IRQ CORE: irq_work_queue function return value not used.

[anish kumar]

From: anish kumar 

As no one is using the return value of irq_work_queue function
it is better to just make it void.

This patch is just a way to understand if there is some future
plan to use it but in any case please let me know the reason.
---
 kernel/irq_work.c |   21 ++---
 1 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/kernel/irq_work.c b/kernel/irq_work.c
index 1588e3b..4a9a44c 100644
--- a/kernel/irq_work.c
+++ b/kernel/irq_work.c
@@ -32,21 +32,21 @@ static DEFINE_PER_CPU(struct llist_head, irq_work_list);
 /*
  * Claim the entry so that no one else will poke at it.
  */
-static bool irq_work_claim(struct irq_work *work)
+static void irq_work_claim(struct irq_work *work)
 {
unsigned long flags, nflags;
 
for (;;) {
flags = work->flags;
if (flags & IRQ_WORK_PENDING)
-   return false;
+   return;
nflags = flags | IRQ_WORK_FLAGS;
if (cmpxchg(>flags, flags, nflags) == flags)
break;
cpu_relax();
}
 
-   return true;
+   return;
 }
 
 void __weak arch_irq_work_raise(void)
@@ -79,15 +79,14 @@ static void __irq_work_queue(struct irq_work *work)
  *
  * Can be re-enqueued while the callback is still in progress.
  */
-bool irq_work_queue(struct irq_work *work)
+void irq_work_queue(struct irq_work *work)
 {
-   if (!irq_work_claim(work)) {
-   /*
-* Already enqueued, can't do!
-*/
-   return false;
-   }
-
+   /*
+* This function either will claim the entry to queue
+* the work or if the work is already queued and is in
+* pending state then it will simply return.
+*/
+   irq_work_claim(work)
__irq_work_queue(work);
return true;
 }
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] irq_work: Fix racy IRQ_WORK_BUSY flag setting

[anish kumar]

On Tue, 2012-10-30 at 14:45 -0400, Steven Rostedt wrote:
> On Wed, 2012-10-31 at 03:33 +0900, anish kumar wrote:
> 
> > > CPU 0 CPU 1
> > > 
> > > data = something flags = IRQ_WORK_BUSY
> > > smp_mb() (implicit with cmpxchg  smp_mb()
> > >   on flags in claim) execute_work (sees data from CPU
> > > 0)
> > > try to claim
> > > 
> > As I understand without the memory barrier proposed by you the situation
> > would be as below:
> > CPU 0 CPU 1
> >  
> > data = something flags = IRQ_WORK_BUSY
> > smp_mb() (implicit with cmpxchg  execute_work (sees data from CPU 0)
> >on flags in claim)
> > _success_ in claiming and goes
> 
> Correct, because it would see the stale value of flags.
> 
> > ahead and execute the work(wrong?)
> >  cmpxchg cause flag to IRQ_WORK_BUSY
> > 
> > Now knows the flag==IRQ_WORK_BUSY
> > 
> > Am I right?
> 
> right.
> 
> > 
> > Probably a stupid question.Why do we return the bool from irq_work_queue
> > when no one bothers to check the return value?Wouldn't it be better if
> > this function is void as used by the users of this function or am I
> > looking at the wrong code.
> 
> Not a stupid question, as I was asking that to myself just earlier
> today. But forgot to mention it as well. Especially, because it makes it
> look like there's a bug in the code. Maybe someday someone will care if
> their work was finished by itself, or some other CPU.
> 
> Probably should just nix the return value.
Can I send a patch to fix this?
> 
> -- Steve
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] irq_work: Fix racy IRQ_WORK_BUSY flag setting

[anish kumar]

On Tue, 2012-10-30 at 22:23 -0400, Steven Rostedt wrote:
> On Wed, 2012-10-31 at 01:36 +0100, Frederic Weisbecker wrote:
> > 2012/10/30 anish kumar :
> > > As I understand without the memory barrier proposed by you the situation
> > > would be as below:
> > > CPU 0 CPU 1
> > >
> > > data = something flags = IRQ_WORK_BUSY
> > > smp_mb() (implicit with cmpxchg  execute_work (sees data from CPU 0)
> > >on flags in claim)
> > > _success_ in claiming and goes
> > > ahead and execute the work(wrong?)
> > >  cmpxchg cause flag to IRQ_WORK_BUSY
> > >
> > > Now knows the flag==IRQ_WORK_BUSY
> > >
> > > Am I right?
> > 
> > (Adding Paul in Cc because I'm again confused with memory barriers)
> > 
> > Actually what I had in mind is rather that CPU 0 fails its claim
> > because it's not seeing the IRQ_WORK_BUSY flag as it should:
> > 
> > 
> > CPU 0 CPU 1
> > 
> > data = something  flags = IRQ_WORK_BUSY
> > cmpxchg() for claim   execute_work (sees data from CPU 0)
> > 
> > CPU 0 should see IRQ_WORK_BUSY but it may not because CPU 1 sets this
> > value in a non-atomic way.
> > 
> > Also, while browsing Paul's perfbook, I realize my changelog is buggy.
> > It seems we can't reliably use memory barriers here because we would
> > be in the following case:
> > 
> > CPU 0  CPU 1
> > 
> > store(work data)store(flags)
> > smp_mb()smp_mb()
> > load(flags)load(work data)
> > 
> > On top of this barrier pairing, we can't make the assumption that, for
> > example, if CPU 1 sees the work data stored in CPU 0 then CPU 0 sees
> > the flags stored in CPU 1.
> > 
> > So now I wonder if cmpxchg() can give us more confidence:
> 
> More confidence over what? The xchg()? They are equivalent (wrt memory
> barriers).
> 
> Here's the issue that currently exists. Let's look at the code:
> 
> 
> /*
>  * Claim the entry so that no one else will poke at it.
>  */
> static bool irq_work_claim(struct irq_work *work)
> {
>   unsigned long flags, nflags;
> 
>   for (;;) {
>   flags = work->flags;
>   if (flags & IRQ_WORK_PENDING)
>   return false;
>   nflags = flags | IRQ_WORK_FLAGS;
nflags = 1 | 3
nflags = 2 | 3
In both cases the result would be same.If I am right then wouldn't this
operation be redundant?
>   if (cmpxchg(>flags, flags, nflags) == flags)
>   break;
>   cpu_relax();
>   }
> 
>   return true;
> }
> 
> and
> 
>   llnode = llist_del_all(this_list);
>   while (llnode != NULL) {
>   work = llist_entry(llnode, struct irq_work, llnode);
> 
>   llnode = llist_next(llnode);
> 
>   /*
>* Clear the PENDING bit, after this point the @work
>* can be re-used.
>*/
>   work->flags = IRQ_WORK_BUSY;
>   work->func(work);
>   /*
>* Clear the BUSY bit and return to the free state if
>* no-one else claimed it meanwhile.
>*/
>   (void)cmpxchg(>flags, IRQ_WORK_BUSY, 0);
>   }
> 
> The irq_work_claim() will only queue its work if it's not already
> pending. If it is pending, then someone is going to process it anyway.
> But once we start running the function, new work needs to be processed
> again.
> 
> Thus we have:
> 
>   CPU 1   CPU 2
>   -   -
>   (flags = 0)
>   cmpxchg(flags, 0, IRQ_WORK_FLAGS)
>   (flags = 3)
>   [...]
> 
>   if (flags & IRQ_WORK_PENDING)
>   return false
>   flags = IRQ_WORK_BUSY
>   (flags = 2)
>   func()
> 
> The above shows the normal case were CPU 2 doesn't need to queue work,
> because its going to be done for it by CPU 1. But...
> 
> 
> 
>   CPU 1   CPU 2
>   -   -
>   (flags = 0)
>   cmpxchg(flags, 0, IRQ_WORK_FLAGS)
>   (flags = 3)
>   [...]
>   flags = IRQ_WORK_BUSY
>   (flags =

Re: [PATCH 2/2] irq_work: Fix racy IRQ_WORK_BUSY flag setting

[anish kumar]

On Tue, 2012-10-30 at 22:23 -0400, Steven Rostedt wrote:
 On Wed, 2012-10-31 at 01:36 +0100, Frederic Weisbecker wrote:
  2012/10/30 anish kumar anish198519851...@gmail.com:
   As I understand without the memory barrier proposed by you the situation
   would be as below:
   CPU 0 CPU 1
  
   data = something flags = IRQ_WORK_BUSY
   smp_mb() (implicit with cmpxchg  execute_work (sees data from CPU 0)
  on flags in claim)
   _success_ in claiming and goes
   ahead and execute the work(wrong?)
cmpxchg cause flag to IRQ_WORK_BUSY
  
   Now knows the flag==IRQ_WORK_BUSY
  
   Am I right?
  
  (Adding Paul in Cc because I'm again confused with memory barriers)
  
  Actually what I had in mind is rather that CPU 0 fails its claim
  because it's not seeing the IRQ_WORK_BUSY flag as it should:
  
  
  CPU 0 CPU 1
  
  data = something  flags = IRQ_WORK_BUSY
  cmpxchg() for claim   execute_work (sees data from CPU 0)
  
  CPU 0 should see IRQ_WORK_BUSY but it may not because CPU 1 sets this
  value in a non-atomic way.
  
  Also, while browsing Paul's perfbook, I realize my changelog is buggy.
  It seems we can't reliably use memory barriers here because we would
  be in the following case:
  
  CPU 0  CPU 1
  
  store(work data)store(flags)
  smp_mb()smp_mb()
  load(flags)load(work data)
  
  On top of this barrier pairing, we can't make the assumption that, for
  example, if CPU 1 sees the work data stored in CPU 0 then CPU 0 sees
  the flags stored in CPU 1.
  
  So now I wonder if cmpxchg() can give us more confidence:
 
 More confidence over what? The xchg()? They are equivalent (wrt memory
 barriers).
 
 Here's the issue that currently exists. Let's look at the code:
 
 
 /*
  * Claim the entry so that no one else will poke at it.
  */
 static bool irq_work_claim(struct irq_work *work)
 {
   unsigned long flags, nflags;
 
   for (;;) {
   flags = work-flags;
   if (flags  IRQ_WORK_PENDING)
   return false;
   nflags = flags | IRQ_WORK_FLAGS;
nflags = 1 | 3
nflags = 2 | 3
In both cases the result would be same.If I am right then wouldn't this
operation be redundant?
   if (cmpxchg(work-flags, flags, nflags) == flags)
   break;
   cpu_relax();
   }
 
   return true;
 }
 
 and
 
   llnode = llist_del_all(this_list);
   while (llnode != NULL) {
   work = llist_entry(llnode, struct irq_work, llnode);
 
   llnode = llist_next(llnode);
 
   /*
* Clear the PENDING bit, after this point the @work
* can be re-used.
*/
   work-flags = IRQ_WORK_BUSY;
   work-func(work);
   /*
* Clear the BUSY bit and return to the free state if
* no-one else claimed it meanwhile.
*/
   (void)cmpxchg(work-flags, IRQ_WORK_BUSY, 0);
   }
 
 The irq_work_claim() will only queue its work if it's not already
 pending. If it is pending, then someone is going to process it anyway.
 But once we start running the function, new work needs to be processed
 again.
 
 Thus we have:
 
   CPU 1   CPU 2
   -   -
   (flags = 0)
   cmpxchg(flags, 0, IRQ_WORK_FLAGS)
   (flags = 3)
   [...]
 
   if (flags  IRQ_WORK_PENDING)
   return false
   flags = IRQ_WORK_BUSY
   (flags = 2)
   func()
 
 The above shows the normal case were CPU 2 doesn't need to queue work,
 because its going to be done for it by CPU 1. But...
 
 
 
   CPU 1   CPU 2
   -   -
   (flags = 0)
   cmpxchg(flags, 0, IRQ_WORK_FLAGS)
   (flags = 3)
   [...]
   flags = IRQ_WORK_BUSY
   (flags = 2)
   func()
   (sees flags = 3)
   if (flags  IRQ_WORK_PENDING)
   return false
   cmpxchg(flags, 2, 0);
   (flags = 0)
 
 
 Here, because we did not do a memory barrier after 
 flags = IRQ_WORK_BUSY, CPU 2 saw stale data and did not queue its work,
 and missed the opportunity. Now if you had this fix with the xchg() as
 you have in your patch, then CPU 2 would not see the stale flags.
 Except, with the code I showed above it still can!
 
   CPU 1   CPU 2
   -   -
   (flags = 0)
   cmpxchg

Re: [PATCH 2/2] irq_work: Fix racy IRQ_WORK_BUSY flag setting

[anish kumar]

On Tue, 2012-10-30 at 14:45 -0400, Steven Rostedt wrote:
 On Wed, 2012-10-31 at 03:33 +0900, anish kumar wrote:
 
   CPU 0 CPU 1
   
   data = something flags = IRQ_WORK_BUSY
   smp_mb() (implicit with cmpxchg  smp_mb()
 on flags in claim) execute_work (sees data from CPU
   0)
   try to claim
   
  As I understand without the memory barrier proposed by you the situation
  would be as below:
  CPU 0 CPU 1
   
  data = something flags = IRQ_WORK_BUSY
  smp_mb() (implicit with cmpxchg  execute_work (sees data from CPU 0)
 on flags in claim)
  _success_ in claiming and goes
 
 Correct, because it would see the stale value of flags.
 
  ahead and execute the work(wrong?)
   cmpxchg cause flag to IRQ_WORK_BUSY
  
  Now knows the flag==IRQ_WORK_BUSY
  
  Am I right?
 
 right.
 
  
  Probably a stupid question.Why do we return the bool from irq_work_queue
  when no one bothers to check the return value?Wouldn't it be better if
  this function is void as used by the users of this function or am I
  looking at the wrong code.
 
 Not a stupid question, as I was asking that to myself just earlier
 today. But forgot to mention it as well. Especially, because it makes it
 look like there's a bug in the code. Maybe someday someone will care if
 their work was finished by itself, or some other CPU.
 
 Probably should just nix the return value.
Can I send a patch to fix this?
 
 -- Steve
 
 


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [RFC]IRQ CORE: irq_work_queue function return value not used.

[anish kumar]

From: anish kumar anish198519851...@gmail.com

As no one is using the return value of irq_work_queue function
it is better to just make it void.

This patch is just a way to understand if there is some future
plan to use it but in any case please let me know the reason.
---
 kernel/irq_work.c |   21 ++---
 1 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/kernel/irq_work.c b/kernel/irq_work.c
index 1588e3b..4a9a44c 100644
--- a/kernel/irq_work.c
+++ b/kernel/irq_work.c
@@ -32,21 +32,21 @@ static DEFINE_PER_CPU(struct llist_head, irq_work_list);
 /*
  * Claim the entry so that no one else will poke at it.
  */
-static bool irq_work_claim(struct irq_work *work)
+static void irq_work_claim(struct irq_work *work)
 {
unsigned long flags, nflags;
 
for (;;) {
flags = work-flags;
if (flags  IRQ_WORK_PENDING)
-   return false;
+   return;
nflags = flags | IRQ_WORK_FLAGS;
if (cmpxchg(work-flags, flags, nflags) == flags)
break;
cpu_relax();
}
 
-   return true;
+   return;
 }
 
 void __weak arch_irq_work_raise(void)
@@ -79,15 +79,14 @@ static void __irq_work_queue(struct irq_work *work)
  *
  * Can be re-enqueued while the callback is still in progress.
  */
-bool irq_work_queue(struct irq_work *work)
+void irq_work_queue(struct irq_work *work)
 {
-   if (!irq_work_claim(work)) {
-   /*
-* Already enqueued, can't do!
-*/
-   return false;
-   }
-
+   /*
+* This function either will claim the entry to queue
+* the work or if the work is already queued and is in
+* pending state then it will simply return.
+*/
+   irq_work_claim(work)
__irq_work_queue(work);
return true;
 }
-- 
1.7.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [RFC]IRQ CORE: irq_work_queue function return value not used.

[anish kumar]

On Wed, 2012-10-31 at 10:15 -0400, Steven Rostedt wrote:
 On Wed, 2012-10-31 at 23:02 +0900, anish kumar wrote:
  From: anish kumar anish198519851...@gmail.com
  
  As no one is using the return value of irq_work_queue function
  it is better to just make it void.
  
  This patch is just a way to understand if there is some future
  plan to use it but in any case please let me know the reason.
  ---
   kernel/irq_work.c |   21 ++---
   1 files changed, 10 insertions(+), 11 deletions(-)
  
  diff --git a/kernel/irq_work.c b/kernel/irq_work.c
  index 1588e3b..4a9a44c 100644
  --- a/kernel/irq_work.c
  +++ b/kernel/irq_work.c
  @@ -32,21 +32,21 @@ static DEFINE_PER_CPU(struct llist_head, irq_work_list);
   /*
* Claim the entry so that no one else will poke at it.
*/
  -static bool irq_work_claim(struct irq_work *work)
  +static void irq_work_claim(struct irq_work *work)
   {
  unsigned long flags, nflags;
   
  for (;;) {
  flags = work-flags;
  if (flags  IRQ_WORK_PENDING)
  -   return false;
  +   return;
  nflags = flags | IRQ_WORK_FLAGS;
  if (cmpxchg(work-flags, flags, nflags) == flags)
  break;
  cpu_relax();
  }
   
  -   return true;
  +   return;
   }
   
   void __weak arch_irq_work_raise(void)
  @@ -79,15 +79,14 @@ static void __irq_work_queue(struct irq_work *work)
*
* Can be re-enqueued while the callback is still in progress.
*/
  -bool irq_work_queue(struct irq_work *work)
  +void irq_work_queue(struct irq_work *work)
   {
  -   if (!irq_work_claim(work)) {
  -   /*
  -* Already enqueued, can't do!
  -*/
  -   return false;
  -   }
  -
  +   /*
  +* This function either will claim the entry to queue
  +* the work or if the work is already queued and is in
  +* pending state then it will simply return.
  +*/
  +   irq_work_claim(work)
 
 Um, no.
 
 If the state was already pending, we will corrupt the llist node of the
 work if we call irq_work_queue(). You must check the return value of
 irq_work_claim() and return if it fails. You can not call
 __irq_work_queue() if irq_work_claim() does not succeed.
 
 The return value of irq_work_queue() can be ignored, but not
 irq_work_claim().
Oh I didn't see that logic properly and rightly pointed out by you, we
should _just_ return instead of queuing the work if the state was
already pending.
 
 -- Steve
 
  __irq_work_queue(work);
  return true;
   }
 
 


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] irq_work: Fix racy IRQ_WORK_BUSY flag setting

[anish kumar]

> The IRQ_WORK_BUSY flag is set right before we execute the
> work. Once this flag value is set, the work enters a
> claimable state again.
> 
> This is necessary because if we want to enqueue a work but we
> fail the claim, we want to ensure that the CPU where that work
> is still pending will see and handle the data we expected the
> work to compute.
> 
> This might not work as expected though because IRQ_WORK_BUSY
> isn't set atomically. By the time a CPU fails a work claim,
> this work may well have been already executed by the CPU where
> it was previously pending.
> 
> Due to the lack of appropriate memory barrier, the IRQ_WORK_BUSY
> flag value may not be visible by the CPU trying to claim while
> the work is executing, and that until we clear the busy bit in
> the work flags using cmpxchg() that implies the full barrier.
> 
> One solution could involve a full barrier between setting
> IRQ_WORK_BUSY flag and the work execution. This way we
> ensure that the work execution site sees the expected data
> and the claim site sees the IRQ_WORK_BUSY:
> 
> CPU 0 CPU 1
> 
> data = something flags = IRQ_WORK_BUSY
> smp_mb() (implicit with cmpxchg  smp_mb()
>   on flags in claim) execute_work (sees data from CPU
> 0)
> try to claim
> 
As I understand without the memory barrier proposed by you the situation
would be as below:
CPU 0 CPU 1
 
data = something flags = IRQ_WORK_BUSY
smp_mb() (implicit with cmpxchg  execute_work (sees data from CPU 0)
   on flags in claim)
_success_ in claiming and goes
ahead and execute the work(wrong?)
 cmpxchg cause flag to IRQ_WORK_BUSY

Now knows the flag==IRQ_WORK_BUSY

Am I right?

Probably a stupid question.Why do we return the bool from irq_work_queue
when no one bothers to check the return value?Wouldn't it be better if
this function is void as used by the users of this function or am I
looking at the wrong code.
> 
> As a shortcut, let's just use xchg() that implies a full memory
> barrier.
> 
> Signed-off-by: Frederic Weisbecker 
> Cc: Peter Zijlstra 
> Cc: Ingo Molnar 
> Cc: Thomas Gleixner 
> Cc: Andrew Morton 
> Cc: Steven Rostedt 
> Cc: Paul Gortmaker 
> ---
>  kernel/irq_work.c |7 +--
>  1 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/irq_work.c b/kernel/irq_work.c
> index 764240a..ea79365 100644
> --- a/kernel/irq_work.c
> +++ b/kernel/irq_work.c
> @@ -130,9 +130,12 @@ void irq_work_run(void)
>  
> /*
>  * Clear the PENDING bit, after this point the @work
> -* can be re-used.
> +* can be re-used. Use xchg to force ordering against
> +* data to process, such that if claiming fails on
> +* another CPU, we see and handle the data it wants
> +* us to process on the work.
>  */
> -   work->flags = IRQ_WORK_BUSY;
> +   xchg(>flags, IRQ_WORK_BUSY);
> work->func(work);
> /*
>  * Clear the BUSY bit and return to the free state if
> -- 
> 1.7.5.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/ 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] irq_work: Fix racy IRQ_WORK_BUSY flag setting

[anish kumar]

 The IRQ_WORK_BUSY flag is set right before we execute the
 work. Once this flag value is set, the work enters a
 claimable state again.
 
 This is necessary because if we want to enqueue a work but we
 fail the claim, we want to ensure that the CPU where that work
 is still pending will see and handle the data we expected the
 work to compute.
 
 This might not work as expected though because IRQ_WORK_BUSY
 isn't set atomically. By the time a CPU fails a work claim,
 this work may well have been already executed by the CPU where
 it was previously pending.
 
 Due to the lack of appropriate memory barrier, the IRQ_WORK_BUSY
 flag value may not be visible by the CPU trying to claim while
 the work is executing, and that until we clear the busy bit in
 the work flags using cmpxchg() that implies the full barrier.
 
 One solution could involve a full barrier between setting
 IRQ_WORK_BUSY flag and the work execution. This way we
 ensure that the work execution site sees the expected data
 and the claim site sees the IRQ_WORK_BUSY:
 
 CPU 0 CPU 1
 
 data = something flags = IRQ_WORK_BUSY
 smp_mb() (implicit with cmpxchg  smp_mb()
   on flags in claim) execute_work (sees data from CPU
 0)
 try to claim
 
As I understand without the memory barrier proposed by you the situation
would be as below:
CPU 0 CPU 1
 
data = something flags = IRQ_WORK_BUSY
smp_mb() (implicit with cmpxchg  execute_work (sees data from CPU 0)
   on flags in claim)
_success_ in claiming and goes
ahead and execute the work(wrong?)
 cmpxchg cause flag to IRQ_WORK_BUSY

Now knows the flag==IRQ_WORK_BUSY

Am I right?

Probably a stupid question.Why do we return the bool from irq_work_queue
when no one bothers to check the return value?Wouldn't it be better if
this function is void as used by the users of this function or am I
looking at the wrong code.
 
 As a shortcut, let's just use xchg() that implies a full memory
 barrier.
 
 Signed-off-by: Frederic Weisbecker fweis...@gmail.com
 Cc: Peter Zijlstra pet...@infradead.org
 Cc: Ingo Molnar mi...@kernel.org
 Cc: Thomas Gleixner t...@linutronix.de
 Cc: Andrew Morton a...@linux-foundation.org
 Cc: Steven Rostedt rost...@goodmis.org
 Cc: Paul Gortmaker paul.gortma...@windriver.com
 ---
  kernel/irq_work.c |7 +--
  1 files changed, 5 insertions(+), 2 deletions(-)
 
 diff --git a/kernel/irq_work.c b/kernel/irq_work.c
 index 764240a..ea79365 100644
 --- a/kernel/irq_work.c
 +++ b/kernel/irq_work.c
 @@ -130,9 +130,12 @@ void irq_work_run(void)
  
 /*
  * Clear the PENDING bit, after this point the @work
 -* can be re-used.
 +* can be re-used. Use xchg to force ordering against
 +* data to process, such that if claiming fails on
 +* another CPU, we see and handle the data it wants
 +* us to process on the work.
  */
 -   work-flags = IRQ_WORK_BUSY;
 +   xchg(work-flags, IRQ_WORK_BUSY);
 work-func(work);
 /*
  * Clear the BUSY bit and return to the free state if
 -- 
 1.7.5.4
 
 --
 To unsubscribe from this list: send the line unsubscribe
 linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/ 

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3] firmware loader: introduce module parameter to customize fw search path

[anish kumar]

On Sat, 2012-10-27 at 09:23 +0800, Ming Lei wrote:
> This patch introduces one module parameter of 'path' in firmware_class
> to support customizing firmware image search path, so that people can
> use its own firmware path if the default built-in paths can't meet their
> demand[1], and the typical usage is passing the below from kernel command
> parameter when 'firmware_class' is built in kernel:
> 
>   firmware_class.path=$CUSTOMIZED_PATH
> 
> [1], https://lkml.org/lkml/2012/10/11/337
> 
> Cc: Linus Torvalds 
> Signed-off-by: Ming Lei 
> ---
> v3
>   - fix one mistake on checking unset firmware path
> 
> v2
>   - take a cleaner approach suggested by Linus
>   - mark the path array as const because it needn't be changed
>   - fix one error in Document about the module name
> 
> v1:
>   - remove kernel boot parameter and only support the feature by
> module parameter as suggested by Greg
> ---
>  Documentation/firmware_class/README |5 +
>  drivers/base/firmware_class.c   |   17 -
>  2 files changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/firmware_class/README 
> b/Documentation/firmware_class/README
> index 815b711..e9fce78 100644
> --- a/Documentation/firmware_class/README
> +++ b/Documentation/firmware_class/README
> @@ -22,12 +22,17 @@
>   - calls request_firmware(_entry, $FIRMWARE, device)
>   - kernel searchs the fimware image with name $FIRMWARE directly
>   in the below search path of root filesystem:
> + User customized search path by module parameter 'path'[1]
>   "/lib/firmware/updates/" UTS_RELEASE,
>   "/lib/firmware/updates",
>   "/lib/firmware/" UTS_RELEASE,
>   "/lib/firmware"
>   - If found, goto 7), else goto 2)
>  
> + [1], the 'path' is a string parameter which length should be less
whose length should be less...
> + than 256, user should pass 'firmware_class.path=$CUSTOMIZED_PATH'
> + if firmware_class is built in kernel(the general situation)
> +
>   2), userspace:
>   - /sys/class/firmware/xxx/{loading,data} appear.
>   - hotplug gets called with a firmware identifier in $FIRMWARE
> diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
> index 8945f4e..62568c2 100644
> --- a/drivers/base/firmware_class.c
> +++ b/drivers/base/firmware_class.c
> @@ -267,13 +267,23 @@ static void fw_free_buf(struct firmware_buf *buf)
>  }
>  
>  /* direct firmware loading support */
> -static const char *fw_path[] = {
> +static char fw_path_para[256];
> +static const char * const fw_path[] = {
> + fw_path_para,
>   "/lib/firmware/updates/" UTS_RELEASE,
>   "/lib/firmware/updates",
>   "/lib/firmware/" UTS_RELEASE,
>   "/lib/firmware"
>  };
>  
> +/*
> + * Typical usage is that passing 'firmware_class.path=$CUSTOMIZED_PATH'
> + * from kernel command because firmware_class is generally built in
do you mean kernel command line?
> + * kernel instead of module.
> + */
> +module_param_string(path, fw_path_para, sizeof(fw_path_para), 0644);
> +MODULE_PARM_DESC(path, "customized firmware image search path with a higher 
> priority than default path");
> +
>  /* Don't inline this: 'struct kstat' is biggish */
>  static noinline long fw_file_size(struct file *file)
>  {
> @@ -315,6 +325,11 @@ static bool fw_get_filesystem_firmware(struct 
> firmware_buf *buf)
>  
>   for (i = 0; i < ARRAY_SIZE(fw_path); i++) {
>   struct file *file;
> +
> + /* skip the unset customized path */
> + if (!fw_path[i][0])
> + continue;
> +
>   snprintf(path, PATH_MAX, "%s/%s", fw_path[i], buf->fw_id);
>  
>   file = filp_open(path, O_RDONLY, 0);


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3] firmware loader: introduce module parameter to customize fw search path

[anish kumar]

On Sat, 2012-10-27 at 09:23 +0800, Ming Lei wrote:
 This patch introduces one module parameter of 'path' in firmware_class
 to support customizing firmware image search path, so that people can
 use its own firmware path if the default built-in paths can't meet their
 demand[1], and the typical usage is passing the below from kernel command
 parameter when 'firmware_class' is built in kernel:
 
   firmware_class.path=$CUSTOMIZED_PATH
 
 [1], https://lkml.org/lkml/2012/10/11/337
 
 Cc: Linus Torvalds torva...@linux-foundation.org
 Signed-off-by: Ming Lei ming@canonical.com
 ---
 v3
   - fix one mistake on checking unset firmware path
 
 v2
   - take a cleaner approach suggested by Linus
   - mark the path array as const because it needn't be changed
   - fix one error in Document about the module name
 
 v1:
   - remove kernel boot parameter and only support the feature by
 module parameter as suggested by Greg
 ---
  Documentation/firmware_class/README |5 +
  drivers/base/firmware_class.c   |   17 -
  2 files changed, 21 insertions(+), 1 deletion(-)
 
 diff --git a/Documentation/firmware_class/README 
 b/Documentation/firmware_class/README
 index 815b711..e9fce78 100644
 --- a/Documentation/firmware_class/README
 +++ b/Documentation/firmware_class/README
 @@ -22,12 +22,17 @@
   - calls request_firmware(fw_entry, $FIRMWARE, device)
   - kernel searchs the fimware image with name $FIRMWARE directly
   in the below search path of root filesystem:
 + User customized search path by module parameter 'path'[1]
   /lib/firmware/updates/ UTS_RELEASE,
   /lib/firmware/updates,
   /lib/firmware/ UTS_RELEASE,
   /lib/firmware
   - If found, goto 7), else goto 2)
  
 + [1], the 'path' is a string parameter which length should be less
whose length should be less...
 + than 256, user should pass 'firmware_class.path=$CUSTOMIZED_PATH'
 + if firmware_class is built in kernel(the general situation)
 +
   2), userspace:
   - /sys/class/firmware/xxx/{loading,data} appear.
   - hotplug gets called with a firmware identifier in $FIRMWARE
 diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
 index 8945f4e..62568c2 100644
 --- a/drivers/base/firmware_class.c
 +++ b/drivers/base/firmware_class.c
 @@ -267,13 +267,23 @@ static void fw_free_buf(struct firmware_buf *buf)
  }
  
  /* direct firmware loading support */
 -static const char *fw_path[] = {
 +static char fw_path_para[256];
 +static const char * const fw_path[] = {
 + fw_path_para,
   /lib/firmware/updates/ UTS_RELEASE,
   /lib/firmware/updates,
   /lib/firmware/ UTS_RELEASE,
   /lib/firmware
  };
  
 +/*
 + * Typical usage is that passing 'firmware_class.path=$CUSTOMIZED_PATH'
 + * from kernel command because firmware_class is generally built in
do you mean kernel command line?
 + * kernel instead of module.
 + */
 +module_param_string(path, fw_path_para, sizeof(fw_path_para), 0644);
 +MODULE_PARM_DESC(path, customized firmware image search path with a higher 
 priority than default path);
 +
  /* Don't inline this: 'struct kstat' is biggish */
  static noinline long fw_file_size(struct file *file)
  {
 @@ -315,6 +325,11 @@ static bool fw_get_filesystem_firmware(struct 
 firmware_buf *buf)
  
   for (i = 0; i  ARRAY_SIZE(fw_path); i++) {
   struct file *file;
 +
 + /* skip the unset customized path */
 + if (!fw_path[i][0])
 + continue;
 +
   snprintf(path, PATH_MAX, %s/%s, fw_path[i], buf-fw_id);
  
   file = filp_open(path, O_RDONLY, 0);


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: irq/manage.c wrong comment( ? )

[anish kumar]

ping...
On Sat, 2012-10-13 at 00:32 +0900, anish kumar wrote:?
> Hello tglx,
> 
> I just found the below inconsistency while going through the code.
> 
> 
> kernel/irq/manage.c
> 
> if (new->flags & IRQF_ONESHOT) {
> /*
>  * The thread_mask for the action is or'ed to
>  * desc->thread_active to indicate that the
>  * IRQF_ONESHOT thread handler has been woken, but not
>  * yet finished. The bit is cleared when a thread
>  * completes. When all threads of a shared interr
> 
> Shouldn't this "desc->thread_active" be desc->threads_active ??


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/