RE: [PATCH v8 2/5] driver core: add deferring probe reason to devices_deferred property

2020-07-10 Thread Ruhl, Michael J
>-Original Message-
>From: dri-devel  On Behalf Of
>Andrzej Hajda
>Sent: Friday, July 10, 2020 11:30 AM
>To: Greg Kroah-Hartman 
>Cc: Jernej Skrabec ; Rafael J. Wysocki
>; Jonas Karlman ; Bartlomiej
>Zolnierkiewicz ; linux-kernel@vger.kernel.org;
>open list:DRM DRIVERS ; Russell King - ARM
>Linux ; Neil Armstrong ;
>Andrzej Hajda ; andy.shevche...@gmail.com; Mark
>Brown ; Laurent Pinchart
>; linux-arm-
>ker...@lists.infradead.org; Marek Szyprowski
>
>Subject: [PATCH v8 2/5] driver core: add deferring probe reason to
>devices_deferred property
>
>/sys/kernel/debug/devices_deferred property contains list of deferred
>devices.
>This list does not contain reason why the driver deferred probe, the patch
>improves it.
>The natural place to set the reason is dev_err_probe function introduced
>recently, ie. if dev_err_probe will be called with -EPROBE_DEFER instead of
>printk the message will be attached to a deferred device and printed when
>user
>reads devices_deferred property.
>
>Signed-off-by: Andrzej Hajda 
>Reviewed-by: Mark Brown 
>Reviewed-by: Javier Martinez Canillas 
>Reviewed-by: Andy Shevchenko 
>Reviewed-by: Rafael J. Wysocki 
>---
>v8:
>- improved commit message
>---
> drivers/base/base.h |  3 +++
> drivers/base/core.c |  8 ++--
> drivers/base/dd.c   | 23 ++-
> 3 files changed, 31 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/base/base.h b/drivers/base/base.h
>index 95c22c0f9036..6954fccab3d7 100644
>--- a/drivers/base/base.h
>+++ b/drivers/base/base.h
>@@ -93,6 +93,7 @@ struct device_private {
>   struct klist_node knode_class;
>   struct list_head deferred_probe;
>   struct device_driver *async_driver;
>+  char *deferred_probe_reason;
>   struct device *device;
>   u8 dead:1;
> };
>@@ -134,6 +135,8 @@ extern void device_release_driver_internal(struct
>device *dev,
> extern void driver_detach(struct device_driver *drv);
> extern int driver_probe_device(struct device_driver *drv, struct device
>*dev);
> extern void driver_deferred_probe_del(struct device *dev);
>+extern void device_set_deferred_probe_reson(const struct device *dev,
>+  struct va_format *vaf);
> static inline int driver_match_device(struct device_driver *drv,
> struct device *dev)
> {
>diff --git a/drivers/base/core.c b/drivers/base/core.c
>index 3a827c82933f..fee047f03681 100644
>--- a/drivers/base/core.c
>+++ b/drivers/base/core.c
>@@ -3963,6 +3963,8 @@ define_dev_printk_level(_dev_info, KERN_INFO);
>  * This helper implements common pattern present in probe functions for
>error
>  * checking: print debug or error message depending if the error value is
>  * -EPROBE_DEFER and propagate error upwards.
>+ * In case of -EPROBE_DEFER it sets also defer probe reason, which can be
>+ * checked later by reading devices_deferred debugfs attribute.
>  * It replaces code sequence:
>  *if (err != -EPROBE_DEFER)
>  *dev_err(dev, ...);
>@@ -3984,10 +3986,12 @@ int dev_err_probe(const struct device *dev, int
>err, const char *fmt, ...)
>   vaf.fmt = fmt;
>   vaf.va = 
>
>-  if (err != -EPROBE_DEFER)
>+  if (err != -EPROBE_DEFER) {
>   dev_err(dev, "error %d: %pV", err, );
>-  else
>+  } else {
>+  device_set_deferred_probe_reson(dev, );
>   dev_dbg(dev, "error %d: %pV", err, );
>+  }
>
>   va_end(args);
>
>diff --git a/drivers/base/dd.c b/drivers/base/dd.c
>index 9a1d940342ac..dd5683b61f74 100644
>--- a/drivers/base/dd.c
>+++ b/drivers/base/dd.c
>@@ -27,6 +27,7 @@
> #include 
> #include 
> #include 
>+#include 
>
> #include "base.h"
> #include "power/power.h"
>@@ -136,6 +137,8 @@ void driver_deferred_probe_del(struct device *dev)
>   if (!list_empty(>p->deferred_probe)) {
>   dev_dbg(dev, "Removed from deferred list\n");
>   list_del_init(>p->deferred_probe);
>+  kfree(dev->p->deferred_probe_reason);
>+  dev->p->deferred_probe_reason = NULL;
>   }
>   mutex_unlock(_probe_mutex);
> }
>@@ -211,6 +214,23 @@ void device_unblock_probing(void)
>   driver_deferred_probe_trigger();
> }
>
>+/**
>+ * device_set_deferred_probe_reson() - Set defer probe reason message
>for device
>+ * @dev: the pointer to the struct device
>+ * @vaf: the pointer to va_format structure with message
>+ */
>+void device_set_deferred_probe_reson(const struct device *dev, struct

Is 'reson'  supposed to be 'reason'?

Same spelling on the above ker

[PATCH v8 2/5] driver core: add deferring probe reason to devices_deferred property

2020-07-10 Thread Andrzej Hajda
/sys/kernel/debug/devices_deferred property contains list of deferred devices.
This list does not contain reason why the driver deferred probe, the patch
improves it.
The natural place to set the reason is dev_err_probe function introduced
recently, ie. if dev_err_probe will be called with -EPROBE_DEFER instead of
printk the message will be attached to a deferred device and printed when user
reads devices_deferred property.

Signed-off-by: Andrzej Hajda 
Reviewed-by: Mark Brown 
Reviewed-by: Javier Martinez Canillas 
Reviewed-by: Andy Shevchenko 
Reviewed-by: Rafael J. Wysocki 
---
v8:
- improved commit message
---
 drivers/base/base.h |  3 +++
 drivers/base/core.c |  8 ++--
 drivers/base/dd.c   | 23 ++-
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/drivers/base/base.h b/drivers/base/base.h
index 95c22c0f9036..6954fccab3d7 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -93,6 +93,7 @@ struct device_private {
struct klist_node knode_class;
struct list_head deferred_probe;
struct device_driver *async_driver;
+   char *deferred_probe_reason;
struct device *device;
u8 dead:1;
 };
@@ -134,6 +135,8 @@ extern void device_release_driver_internal(struct device 
*dev,
 extern void driver_detach(struct device_driver *drv);
 extern int driver_probe_device(struct device_driver *drv, struct device *dev);
 extern void driver_deferred_probe_del(struct device *dev);
+extern void device_set_deferred_probe_reson(const struct device *dev,
+   struct va_format *vaf);
 static inline int driver_match_device(struct device_driver *drv,
  struct device *dev)
 {
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 3a827c82933f..fee047f03681 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -3963,6 +3963,8 @@ define_dev_printk_level(_dev_info, KERN_INFO);
  * This helper implements common pattern present in probe functions for error
  * checking: print debug or error message depending if the error value is
  * -EPROBE_DEFER and propagate error upwards.
+ * In case of -EPROBE_DEFER it sets also defer probe reason, which can be
+ * checked later by reading devices_deferred debugfs attribute.
  * It replaces code sequence:
  * if (err != -EPROBE_DEFER)
  * dev_err(dev, ...);
@@ -3984,10 +3986,12 @@ int dev_err_probe(const struct device *dev, int err, 
const char *fmt, ...)
vaf.fmt = fmt;
vaf.va = 
 
-   if (err != -EPROBE_DEFER)
+   if (err != -EPROBE_DEFER) {
dev_err(dev, "error %d: %pV", err, );
-   else
+   } else {
+   device_set_deferred_probe_reson(dev, );
dev_dbg(dev, "error %d: %pV", err, );
+   }
 
va_end(args);
 
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 9a1d940342ac..dd5683b61f74 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "base.h"
 #include "power/power.h"
@@ -136,6 +137,8 @@ void driver_deferred_probe_del(struct device *dev)
if (!list_empty(>p->deferred_probe)) {
dev_dbg(dev, "Removed from deferred list\n");
list_del_init(>p->deferred_probe);
+   kfree(dev->p->deferred_probe_reason);
+   dev->p->deferred_probe_reason = NULL;
}
mutex_unlock(_probe_mutex);
 }
@@ -211,6 +214,23 @@ void device_unblock_probing(void)
driver_deferred_probe_trigger();
 }
 
+/**
+ * device_set_deferred_probe_reson() - Set defer probe reason message for 
device
+ * @dev: the pointer to the struct device
+ * @vaf: the pointer to va_format structure with message
+ */
+void device_set_deferred_probe_reson(const struct device *dev, struct 
va_format *vaf)
+{
+   const char *drv = dev_driver_string(dev);
+
+   mutex_lock(_probe_mutex);
+
+   kfree(dev->p->deferred_probe_reason);
+   dev->p->deferred_probe_reason = kasprintf(GFP_KERNEL, "%s: %pV", drv, 
vaf);
+
+   mutex_unlock(_probe_mutex);
+}
+
 /*
  * deferred_devs_show() - Show the devices in the deferred probe pending list.
  */
@@ -221,7 +241,8 @@ static int deferred_devs_show(struct seq_file *s, void 
*data)
mutex_lock(_probe_mutex);
 
list_for_each_entry(curr, _probe_pending_list, deferred_probe)
-   seq_printf(s, "%s\n", dev_name(curr->device));
+   seq_printf(s, "%s\t%s", dev_name(curr->device),
+  curr->device->p->deferred_probe_reason ?: "\n");
 
mutex_unlock(_probe_mutex);
 
-- 
2.17.1



Re: [PATCH v8 2/5] driver core: add deferring probe reason to devices_deferred property

2020-07-10 Thread Andrzej Hajda


On 10.07.2020 15:31, Greg Kroah-Hartman wrote:
> On Thu, Jul 02, 2020 at 03:44:21PM +0200, Andrzej Hajda wrote:
>> /sys/kernel/debug/devices_deferred property contains list of deferred 
>> devices.
>> This list does not contain reason why the driver deferred probe, the patch
>> improves it.
>> The natural place to set the reason is dev_err_probe function introduced
>> recently, ie. if dev_err_probe will be called with -EPROBE_DEFER instead of
>> printk the message will be attached to a deferred device and printed when 
>> user
>> reads devices_deferred property.
>>
>> Signed-off-by: Andrzej Hajda 
>> Reviewed-by: Mark Brown 
>> Reviewed-by: Javier Martinez Canillas 
>> Reviewed-by: Andy Shevchenko 
>> Reviewed-by: Rafael J. Wysocki 
>> ---
>> v8:
>> - improved commit message
> I'm totally confused by this series.  Can you resend the whole thing,
> as a full series, not just random individual patches in the series
> incremented?  It's a pain to try to fish them all out as to which is the
> "latest" with all of the needed reviewed by lines :(


v7 is the latest except this one,which contains only commit message change.

Anyway I will send v8 to make things simple.


Regards

Andrzej


>
> thanks,
>
> greg k-h
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://protect2.fireeye.com/v1/url?k=563dadd0-0bf16175-563c269f-0cc47a30d446-7237066d193b28b5=1=54779b9e-347e-4d0c-9845-da31d4cce7e4=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel
>


Re: [PATCH v8 2/5] driver core: add deferring probe reason to devices_deferred property

2020-07-10 Thread Greg Kroah-Hartman
On Thu, Jul 02, 2020 at 03:44:21PM +0200, Andrzej Hajda wrote:
> /sys/kernel/debug/devices_deferred property contains list of deferred devices.
> This list does not contain reason why the driver deferred probe, the patch
> improves it.
> The natural place to set the reason is dev_err_probe function introduced
> recently, ie. if dev_err_probe will be called with -EPROBE_DEFER instead of
> printk the message will be attached to a deferred device and printed when user
> reads devices_deferred property.
> 
> Signed-off-by: Andrzej Hajda 
> Reviewed-by: Mark Brown 
> Reviewed-by: Javier Martinez Canillas 
> Reviewed-by: Andy Shevchenko 
> Reviewed-by: Rafael J. Wysocki 
> ---
> v8:
> - improved commit message

I'm totally confused by this series.  Can you resend the whole thing,
as a full series, not just random individual patches in the series
incremented?  It's a pain to try to fish them all out as to which is the
"latest" with all of the needed reviewed by lines :(

thanks,

greg k-h


[PATCH v8 2/5] driver core: add deferring probe reason to devices_deferred property

2020-07-02 Thread Andrzej Hajda
/sys/kernel/debug/devices_deferred property contains list of deferred devices.
This list does not contain reason why the driver deferred probe, the patch
improves it.
The natural place to set the reason is dev_err_probe function introduced
recently, ie. if dev_err_probe will be called with -EPROBE_DEFER instead of
printk the message will be attached to a deferred device and printed when user
reads devices_deferred property.

Signed-off-by: Andrzej Hajda 
Reviewed-by: Mark Brown 
Reviewed-by: Javier Martinez Canillas 
Reviewed-by: Andy Shevchenko 
Reviewed-by: Rafael J. Wysocki 
---
v8:
- improved commit message
---
 drivers/base/base.h |  3 +++
 drivers/base/core.c |  8 ++--
 drivers/base/dd.c   | 23 ++-
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/drivers/base/base.h b/drivers/base/base.h
index 95c22c0f9036..6954fccab3d7 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -93,6 +93,7 @@ struct device_private {
struct klist_node knode_class;
struct list_head deferred_probe;
struct device_driver *async_driver;
+   char *deferred_probe_reason;
struct device *device;
u8 dead:1;
 };
@@ -134,6 +135,8 @@ extern void device_release_driver_internal(struct device 
*dev,
 extern void driver_detach(struct device_driver *drv);
 extern int driver_probe_device(struct device_driver *drv, struct device *dev);
 extern void driver_deferred_probe_del(struct device *dev);
+extern void device_set_deferred_probe_reson(const struct device *dev,
+   struct va_format *vaf);
 static inline int driver_match_device(struct device_driver *drv,
  struct device *dev)
 {
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 3a827c82933f..fee047f03681 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -3963,6 +3963,8 @@ define_dev_printk_level(_dev_info, KERN_INFO);
  * This helper implements common pattern present in probe functions for error
  * checking: print debug or error message depending if the error value is
  * -EPROBE_DEFER and propagate error upwards.
+ * In case of -EPROBE_DEFER it sets also defer probe reason, which can be
+ * checked later by reading devices_deferred debugfs attribute.
  * It replaces code sequence:
  * if (err != -EPROBE_DEFER)
  * dev_err(dev, ...);
@@ -3984,10 +3986,12 @@ int dev_err_probe(const struct device *dev, int err, 
const char *fmt, ...)
vaf.fmt = fmt;
vaf.va = 
 
-   if (err != -EPROBE_DEFER)
+   if (err != -EPROBE_DEFER) {
dev_err(dev, "error %d: %pV", err, );
-   else
+   } else {
+   device_set_deferred_probe_reson(dev, );
dev_dbg(dev, "error %d: %pV", err, );
+   }
 
va_end(args);
 
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 9a1d940342ac..dd5683b61f74 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "base.h"
 #include "power/power.h"
@@ -136,6 +137,8 @@ void driver_deferred_probe_del(struct device *dev)
if (!list_empty(>p->deferred_probe)) {
dev_dbg(dev, "Removed from deferred list\n");
list_del_init(>p->deferred_probe);
+   kfree(dev->p->deferred_probe_reason);
+   dev->p->deferred_probe_reason = NULL;
}
mutex_unlock(_probe_mutex);
 }
@@ -211,6 +214,23 @@ void device_unblock_probing(void)
driver_deferred_probe_trigger();
 }
 
+/**
+ * device_set_deferred_probe_reson() - Set defer probe reason message for 
device
+ * @dev: the pointer to the struct device
+ * @vaf: the pointer to va_format structure with message
+ */
+void device_set_deferred_probe_reson(const struct device *dev, struct 
va_format *vaf)
+{
+   const char *drv = dev_driver_string(dev);
+
+   mutex_lock(_probe_mutex);
+
+   kfree(dev->p->deferred_probe_reason);
+   dev->p->deferred_probe_reason = kasprintf(GFP_KERNEL, "%s: %pV", drv, 
vaf);
+
+   mutex_unlock(_probe_mutex);
+}
+
 /*
  * deferred_devs_show() - Show the devices in the deferred probe pending list.
  */
@@ -221,7 +241,8 @@ static int deferred_devs_show(struct seq_file *s, void 
*data)
mutex_lock(_probe_mutex);
 
list_for_each_entry(curr, _probe_pending_list, deferred_probe)
-   seq_printf(s, "%s\n", dev_name(curr->device));
+   seq_printf(s, "%s\t%s", dev_name(curr->device),
+  curr->device->p->deferred_probe_reason ?: "\n");
 
mutex_unlock(_probe_mutex);
 
-- 
2.17.1