diff mbox series

[v2,5/7] libpdbg: Rescan fsi bus only once

Message ID 20200610052426.150225-6-amitay@ozlabs.org
State Accepted
Headers show
Series Make kernel fsi driver generic | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success Successfully applied on branch master (86851b290ac1771b9a6fb0d5238ebf459ea80a21)
snowpatch_ozlabs/build-multiarch success Test build-multiarch on branch master

Commit Message

Amitay Isaacs June 10, 2020, 5:24 a.m. UTC
On rescan, kernel driver re-creates all the slave devices.  This
invalidates all the slave deviced opened before the scan.  So avoid
scanning fsi bus, if any of the devices are opened.

Signed-off-by: Amitay Isaacs <amitay@ozlabs.org>
---
 libpdbg/kernel.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

Comments

Alistair Popple June 15, 2020, 7:31 a.m. UTC | #1
Looks good to me. It's up to higher level system logic to manage scanning of 
the busses in relation to power on/off so in theory pdbg shouldn't have to 
initiate a rescan anyway.

Reviewed-by: Alistair Popple <alistair@popple.id.au>

On Wednesday, 10 June 2020 3:24:24 PM AEST Amitay Isaacs wrote:
> On rescan, kernel driver re-creates all the slave devices.  This
> invalidates all the slave deviced opened before the scan.  So avoid
> scanning fsi bus, if any of the devices are opened.
> 
> Signed-off-by: Amitay Isaacs <amitay@ozlabs.org>
> ---
>  libpdbg/kernel.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/libpdbg/kernel.c b/libpdbg/kernel.c
> index 2cc81f2..c63ef93 100644
> --- a/libpdbg/kernel.c
> +++ b/libpdbg/kernel.c
> @@ -149,6 +149,7 @@ int kernel_fsi_probe(struct pdbg_target *target)
>  	const char *kernel_path = kernel_get_fsi_path();
>  	const char *fsi_path;
>  	char *path;
> +	static bool first_probe = true;
>  
>  	if (!kernel_path)
>  		return -1;
> @@ -163,23 +164,30 @@ int kernel_fsi_probe(struct pdbg_target *target)
>  	}
>  
>  	while (tries) {
> -		/* Open first raw device */
>  		fsi->fd = open(path, O_RDWR | O_SYNC);
>  		if (fsi->fd >= 0) {
>  			free(path);
> +			first_probe = false;
>  			return 0;
>  		}
>  		tries--;
>  
> -		/* Scan */
> -		kernel_fsi_scan_devices();
> -		sleep(1);
> -	}
> -	if (fsi->fd < 0) {
> -		PR_ERROR("Unable to open %s\n", path);
> -		free(path);
> +		/*
> +		 * On fsi bus rescan, kernel re-creates all the slave device
> +		 * entries.  It means any currently open devices will be
> +		 * invalid and need to be re-opened.  So avoid scanning if
> +		 * some devices are already probed.
> +		 */
> +		if (first_probe) {
> +			kernel_fsi_scan_devices();
> +			sleep(1);
> +		} else {
> +			break;
> +		}
>  	}
>  
> +	PR_ERROR("Unable to open %s\n", path);
> +	free(path);
>  	return -1;
>  }
>  
>
Joel Stanley June 16, 2020, 1:32 a.m. UTC | #2
On Wed, 10 Jun 2020 at 05:25, Amitay Isaacs <amitay@ozlabs.org> wrote:
>
> On rescan, kernel driver re-creates all the slave devices.  This
> invalidates all the slave deviced opened before the scan.  So avoid
> scanning fsi bus, if any of the devices are opened.
>
> Signed-off-by: Amitay Isaacs <amitay@ozlabs.org>

This looks good.

Reviewed-by: Joel Stanley <joel@jms.id.au>


> ---
>  libpdbg/kernel.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/libpdbg/kernel.c b/libpdbg/kernel.c
> index 2cc81f2..c63ef93 100644
> --- a/libpdbg/kernel.c
> +++ b/libpdbg/kernel.c
> @@ -149,6 +149,7 @@ int kernel_fsi_probe(struct pdbg_target *target)
>         const char *kernel_path = kernel_get_fsi_path();
>         const char *fsi_path;
>         char *path;
> +       static bool first_probe = true;
>
>         if (!kernel_path)
>                 return -1;
> @@ -163,23 +164,30 @@ int kernel_fsi_probe(struct pdbg_target *target)
>         }
>
>         while (tries) {
> -               /* Open first raw device */
>                 fsi->fd = open(path, O_RDWR | O_SYNC);
>                 if (fsi->fd >= 0) {
>                         free(path);
> +                       first_probe = false;
>                         return 0;
>                 }
>                 tries--;
>
> -               /* Scan */
> -               kernel_fsi_scan_devices();
> -               sleep(1);
> -       }
> -       if (fsi->fd < 0) {
> -               PR_ERROR("Unable to open %s\n", path);
> -               free(path);
> +               /*
> +                * On fsi bus rescan, kernel re-creates all the slave device
> +                * entries.  It means any currently open devices will be
> +                * invalid and need to be re-opened.  So avoid scanning if
> +                * some devices are already probed.
> +                */
> +               if (first_probe) {
> +                       kernel_fsi_scan_devices();
> +                       sleep(1);
> +               } else {
> +                       break;
> +               }
>         }
>
> +       PR_ERROR("Unable to open %s\n", path);
> +       free(path);
>         return -1;
>  }
>
> --
> 2.26.2
>
> --
> Pdbg mailing list
> Pdbg@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/pdbg
diff mbox series

Patch

diff --git a/libpdbg/kernel.c b/libpdbg/kernel.c
index 2cc81f2..c63ef93 100644
--- a/libpdbg/kernel.c
+++ b/libpdbg/kernel.c
@@ -149,6 +149,7 @@  int kernel_fsi_probe(struct pdbg_target *target)
 	const char *kernel_path = kernel_get_fsi_path();
 	const char *fsi_path;
 	char *path;
+	static bool first_probe = true;
 
 	if (!kernel_path)
 		return -1;
@@ -163,23 +164,30 @@  int kernel_fsi_probe(struct pdbg_target *target)
 	}
 
 	while (tries) {
-		/* Open first raw device */
 		fsi->fd = open(path, O_RDWR | O_SYNC);
 		if (fsi->fd >= 0) {
 			free(path);
+			first_probe = false;
 			return 0;
 		}
 		tries--;
 
-		/* Scan */
-		kernel_fsi_scan_devices();
-		sleep(1);
-	}
-	if (fsi->fd < 0) {
-		PR_ERROR("Unable to open %s\n", path);
-		free(path);
+		/*
+		 * On fsi bus rescan, kernel re-creates all the slave device
+		 * entries.  It means any currently open devices will be
+		 * invalid and need to be re-opened.  So avoid scanning if
+		 * some devices are already probed.
+		 */
+		if (first_probe) {
+			kernel_fsi_scan_devices();
+			sleep(1);
+		} else {
+			break;
+		}
 	}
 
+	PR_ERROR("Unable to open %s\n", path);
+	free(path);
 	return -1;
 }