| Message ID | 20200610052426.150225-6-amitay@ozlabs.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Make kernel fsi driver generic | expand |
| 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 |
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; > } > >
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 --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; }
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(-)