mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH v1 1/2] add device state flags and add error state
@ 2021-06-06 12:23 Oleksij Rempel
  2021-06-06 12:24 ` [PATCH v1 2/2] devinfo: print only devices with errors Oleksij Rempel
  2021-06-21  8:07 ` [PATCH v1 1/2] add device state flags and add error state Ahmad Fatoum
  0 siblings, 2 replies; 6+ messages in thread
From: Oleksij Rempel @ 2021-06-06 12:23 UTC (permalink / raw)
  To: barebox; +Cc: Oleksij Rempel

Add state flags for each registered device and set error state on each
dev_err print.
This states can be used by users to identify erroneous device.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 include/driver.h | 13 +++++++++++++
 include/printk.h |  5 ++++-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/include/driver.h b/include/driver.h
index d84fe35d50..b64a8e258c 100644
--- a/include/driver.h
+++ b/include/driver.h
@@ -88,6 +88,9 @@ struct device_d {
 	 * when the driver should actually detect client devices
 	 */
 	int     (*detect) (struct device_d *);
+
+#define DEV_ERR			BIT(0)
+	u32	run_flags;
 };
 
 /** @brief Describes a driver present in the system */
@@ -361,6 +364,16 @@ static inline int dev_close_default(struct device_d *dev, struct filep *f)
 	return 0;
 }
 
+static inline void dev_set_err(struct device_d *dev)
+{
+	dev->run_flags |= DEV_ERR;
+}
+
+static inline bool dev_have_err(struct device_d *dev)
+{
+	return !!(dev->run_flags & DEV_ERR);
+}
+
 struct bus_type {
 	char *name;
 	int (*match)(struct device_d *dev, struct driver_d *drv);
diff --git a/include/printk.h b/include/printk.h
index f83ad3bf07..6c563be3e6 100644
--- a/include/printk.h
+++ b/include/printk.h
@@ -61,7 +61,10 @@ static inline int pr_print(int level, const char *format, ...)
 #define dev_crit(dev, format, arg...)		\
 	__dev_printf(2, (dev) , format , ## arg)
 #define dev_err(dev, format, arg...)		\
-	__dev_printf(3, (dev) , format , ## arg)
+	({ \
+		dev_set_err(dev); \
+		__dev_printf(3, (dev) , format , ## arg); \
+	})
 #define dev_warn(dev, format, arg...)		\
 	__dev_printf(4, (dev) , format , ## arg)
 #define dev_notice(dev, format, arg...)		\
-- 
2.29.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v1 2/2] devinfo: print only devices with errors
  2021-06-06 12:23 [PATCH v1 1/2] add device state flags and add error state Oleksij Rempel
@ 2021-06-06 12:24 ` Oleksij Rempel
  2021-06-16  7:59   ` Sascha Hauer
  2021-06-21  8:07 ` [PATCH v1 1/2] add device state flags and add error state Ahmad Fatoum
  1 sibling, 1 reply; 6+ messages in thread
From: Oleksij Rempel @ 2021-06-06 12:24 UTC (permalink / raw)
  To: barebox; +Cc: Oleksij Rempel

Make use of device state flags and print only devices with errors.
The result will look like this:
barebox@RIoTboard i.MX6S:/ devinfo -e
`-- 2188000.ethernet@2188000.of <-- (error!)
   `-- miibus0
      `-- mdio0-phy04
         `-- 0x00000000-0x0000003f (  64 Bytes): /dev/mdio0-phy04
   `-- eth0

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 commands/devinfo.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/commands/devinfo.c b/commands/devinfo.c
index 2e2e48e42c..af57cac522 100644
--- a/commands/devinfo.c
+++ b/commands/devinfo.c
@@ -5,6 +5,7 @@
 #include <common.h>
 #include <complete.h>
 #include <driver.h>
+#include <getopt.h>
 
 static int do_devinfo_subtree(struct device_d *dev, int depth)
 {
@@ -16,6 +17,9 @@ static int do_devinfo_subtree(struct device_d *dev, int depth)
 		printf("   ");
 
 	printf("`-- %s", dev_name(dev));
+	if (dev_have_err(dev))
+		printf(" <-- (error!)");
+
 	if (!list_empty(&dev->cdevs)) {
 		printf("\n");
 		list_for_each_entry(cdev, &dev->cdevs, devices_list) {
@@ -43,6 +47,15 @@ static int do_devinfo_subtree(struct device_d *dev, int depth)
 	return 0;
 }
 
+static int do_devinfo_err(void)
+{
+	struct device_d *dev;
+
+	for_each_device(dev)
+		if (dev_have_err(dev))
+			do_devinfo_subtree(dev, 0);
+	return 0;
+}
 
 static int do_devinfo(int argc, char *argv[])
 {
@@ -58,6 +71,23 @@ static int do_devinfo(int argc, char *argv[])
 				do_devinfo_subtree(dev, 0);
 		}
 	} else {
+		bool dev_err = false;
+		int opt;
+
+		while ((opt = getopt(argc, argv, "e")) > 0) {
+			switch (opt) {
+			case 'e':
+				dev_err = true;
+				break;
+			default:
+				return COMMAND_ERROR_USAGE;
+			}
+		}
+
+		if (dev_err) {
+			return do_devinfo_err();
+		}
+
 		dev = get_device_by_name(argv[1]);
 		if (!dev)
 			return -ENODEV;
-- 
2.29.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 2/2] devinfo: print only devices with errors
  2021-06-06 12:24 ` [PATCH v1 2/2] devinfo: print only devices with errors Oleksij Rempel
@ 2021-06-16  7:59   ` Sascha Hauer
  2021-06-16  8:19     ` Oleksij Rempel
  0 siblings, 1 reply; 6+ messages in thread
From: Sascha Hauer @ 2021-06-16  7:59 UTC (permalink / raw)
  To: Oleksij Rempel; +Cc: barebox

Hi Oleksij,

On Sun, Jun 06, 2021 at 02:24:00PM +0200, Oleksij Rempel wrote:
> Make use of device state flags and print only devices with errors.
> The result will look like this:
> barebox@RIoTboard i.MX6S:/ devinfo -e
> `-- 2188000.ethernet@2188000.of <-- (error!)
>    `-- miibus0
>       `-- mdio0-phy04
>          `-- 0x00000000-0x0000003f (  64 Bytes): /dev/mdio0-phy04
>    `-- eth0

What advantages brings that over doing a dmesg and looking at the
output?

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 2/2] devinfo: print only devices with errors
  2021-06-16  7:59   ` Sascha Hauer
@ 2021-06-16  8:19     ` Oleksij Rempel
  0 siblings, 0 replies; 6+ messages in thread
From: Oleksij Rempel @ 2021-06-16  8:19 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

On Wed, Jun 16, 2021 at 09:59:03AM +0200, Sascha Hauer wrote:
> Hi Oleksij,
> 
> On Sun, Jun 06, 2021 at 02:24:00PM +0200, Oleksij Rempel wrote:
> > Make use of device state flags and print only devices with errors.
> > The result will look like this:
> > barebox@RIoTboard i.MX6S:/ devinfo -e
> > `-- 2188000.ethernet@2188000.of <-- (error!)
> >    `-- miibus0
> >       `-- mdio0-phy04
> >          `-- 0x00000000-0x0000003f (  64 Bytes): /dev/mdio0-phy04
> >    `-- eth0
> 
> What advantages brings that over doing a dmesg and looking at the
> output?

It makes it possible to add additional handlers. For example, change
heartbeat pattern if some errors was detected. Or add board specific
selftest to check if listed devices was initialized without errors.

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 1/2] add device state flags and add error state
  2021-06-06 12:23 [PATCH v1 1/2] add device state flags and add error state Oleksij Rempel
  2021-06-06 12:24 ` [PATCH v1 2/2] devinfo: print only devices with errors Oleksij Rempel
@ 2021-06-21  8:07 ` Ahmad Fatoum
  2021-06-21  8:24   ` Bartosz Bilas
  1 sibling, 1 reply; 6+ messages in thread
From: Ahmad Fatoum @ 2021-06-21  8:07 UTC (permalink / raw)
  To: Oleksij Rempel, barebox

On 06.06.21 14:23, Oleksij Rempel wrote:
> Add state flags for each registered device and set error state on each
> dev_err print.
> This states can be used by users to identify erroneous device.

I thought this over a bit. We have dev_err calls when:
 
 - probes are permanently deferred
 - have actual probe (!= -ENODEV, != -ENXIO, != -EPROBE_DEFER)
 - after probe something fails (e.g. you got the regulator, but couldn't enable it)

So, I think the hook point is appropriate. Board code could extend this
and get a device and do some extra tests and use dev_set_err as appropriate.

So feel free to add:

Acked-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  include/driver.h | 13 +++++++++++++
>  include/printk.h |  5 ++++-
>  2 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/include/driver.h b/include/driver.h
> index d84fe35d50..b64a8e258c 100644
> --- a/include/driver.h
> +++ b/include/driver.h
> @@ -88,6 +88,9 @@ struct device_d {
>  	 * when the driver should actually detect client devices
>  	 */
>  	int     (*detect) (struct device_d *);
> +
> +#define DEV_ERR			BIT(0)
> +	u32	run_flags;

Just turn it into a bit field instead?

>  };
>  
>  /** @brief Describes a driver present in the system */
> @@ -361,6 +364,16 @@ static inline int dev_close_default(struct device_d *dev, struct filep *f)
>  	return 0;
>  }
>  
> +static inline void dev_set_err(struct device_d *dev)
> +{
> +	dev->run_flags |= DEV_ERR;
> +}
> +
> +static inline bool dev_have_err(struct device_d *dev)
> +{
> +	return !!(dev->run_flags & DEV_ERR);
> +}
> +
>  struct bus_type {
>  	char *name;
>  	int (*match)(struct device_d *dev, struct driver_d *drv);
> diff --git a/include/printk.h b/include/printk.h
> index f83ad3bf07..6c563be3e6 100644
> --- a/include/printk.h
> +++ b/include/printk.h
> @@ -61,7 +61,10 @@ static inline int pr_print(int level, const char *format, ...)
>  #define dev_crit(dev, format, arg...)		\
>  	__dev_printf(2, (dev) , format , ## arg)
>  #define dev_err(dev, format, arg...)		\
> -	__dev_printf(3, (dev) , format , ## arg)
> +	({ \

You're already using statement expressions, so you could add a
		struct device_d *__dev = (dev);
here and use that.

> +		dev_set_err(dev); \
> +		__dev_printf(3, (dev) , format , ## arg); \
> +	})
>  #define dev_warn(dev, format, arg...)		\
>  	__dev_printf(4, (dev) , format , ## arg)
>  #define dev_notice(dev, format, arg...)		\
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1 1/2] add device state flags and add error state
  2021-06-21  8:07 ` [PATCH v1 1/2] add device state flags and add error state Ahmad Fatoum
@ 2021-06-21  8:24   ` Bartosz Bilas
  0 siblings, 0 replies; 6+ messages in thread
From: Bartosz Bilas @ 2021-06-21  8:24 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox, Oleksij Rempel

Hello,

On 21.06.2021 10:07, Ahmad Fatoum wrote:
> On 06.06.21 14:23, Oleksij Rempel wrote:
>> Add state flags for each registered device and set error state on each
>> dev_err print.
>> This states can be used by users to identify erroneous device.
> I thought this over a bit. We have dev_err calls when:
>   
>   - probes are permanently deferred
>   - have actual probe (!= -ENODEV, != -ENXIO, != -EPROBE_DEFER)
>   - after probe something fails (e.g. you got the regulator, but couldn't enable it)
>
> So, I think the hook point is appropriate. Board code could extend this
> and get a device and do some extra tests and use dev_set_err as appropriate.
>
> So feel free to add:
>
> Acked-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>
>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>> ---
>>   include/driver.h | 13 +++++++++++++
>>   include/printk.h |  5 ++++-
>>   2 files changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/driver.h b/include/driver.h
>> index d84fe35d50..b64a8e258c 100644
>> --- a/include/driver.h
>> +++ b/include/driver.h
>> @@ -88,6 +88,9 @@ struct device_d {
>>   	 * when the driver should actually detect client devices
>>   	 */
>>   	int     (*detect) (struct device_d *);
>> +
>> +#define DEV_ERR			BIT(0)
>> +	u32	run_flags;
> Just turn it into a bit field instead?
>
>>   };
>>   
>>   /** @brief Describes a driver present in the system */
>> @@ -361,6 +364,16 @@ static inline int dev_close_default(struct device_d *dev, struct filep *f)
>>   	return 0;
>>   }
>>   
>> +static inline void dev_set_err(struct device_d *dev)
>> +{
>> +	dev->run_flags |= DEV_ERR;
>> +}
>> +
>> +static inline bool dev_have_err(struct device_d *dev)
>> +{
>> +	return !!(dev->run_flags & DEV_ERR);

Isn't return dev->run_flags & DEV_ERR enough here?

Best
Bartek
>> +}
>> +
>>   struct bus_type {
>>   	char *name;
>>   	int (*match)(struct device_d *dev, struct driver_d *drv);
>> diff --git a/include/printk.h b/include/printk.h
>> index f83ad3bf07..6c563be3e6 100644
>> --- a/include/printk.h
>> +++ b/include/printk.h
>> @@ -61,7 +61,10 @@ static inline int pr_print(int level, const char *format, ...)
>>   #define dev_crit(dev, format, arg...)		\
>>   	__dev_printf(2, (dev) , format , ## arg)
>>   #define dev_err(dev, format, arg...)		\
>> -	__dev_printf(3, (dev) , format , ## arg)
>> +	({ \
> You're already using statement expressions, so you could add a
> 		struct device_d *__dev = (dev);
> here and use that.
>
>> +		dev_set_err(dev); \
>> +		__dev_printf(3, (dev) , format , ## arg); \
>> +	})
>>   #define dev_warn(dev, format, arg...)		\
>>   	__dev_printf(4, (dev) , format , ## arg)
>>   #define dev_notice(dev, format, arg...)		\
>>

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-06-21  8:25 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-06 12:23 [PATCH v1 1/2] add device state flags and add error state Oleksij Rempel
2021-06-06 12:24 ` [PATCH v1 2/2] devinfo: print only devices with errors Oleksij Rempel
2021-06-16  7:59   ` Sascha Hauer
2021-06-16  8:19     ` Oleksij Rempel
2021-06-21  8:07 ` [PATCH v1 1/2] add device state flags and add error state Ahmad Fatoum
2021-06-21  8:24   ` Bartosz Bilas

mail archive of the barebox mailing list

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://lore.barebox.org/barebox/0 barebox/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 barebox barebox/ https://lore.barebox.org/barebox \
		barebox@lists.infradead.org barebox@lists.infradead.org
	public-inbox-index barebox

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git