| Message ID | 20200610052426.150225-2-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 |
On Wednesday, 10 June 2020 3:24:20 PM AEST Amitay Isaacs wrote: > This allows to have different fd for different fsi targets. Also, we > can close the fd cleanly in release procedure. > > Signed-off-by: Amitay Isaacs <amitay@ozlabs.org> > --- > libpdbg/hwunit.h | 1 + > libpdbg/kernel.c | 87 +++++++++++++++++++++++------------------------- > 2 files changed, 43 insertions(+), 45 deletions(-) > > diff --git a/libpdbg/hwunit.h b/libpdbg/hwunit.h > index 61aea0f..e3a8426 100644 > --- a/libpdbg/hwunit.h > +++ b/libpdbg/hwunit.h > @@ -112,6 +112,7 @@ struct fsi { > int (*read)(struct fsi *, uint32_t, uint32_t *); > int (*write)(struct fsi *, uint32_t, uint32_t); > enum chip_type chip_type; > + int fd; > }; > #define target_to_fsi(x) container_of(x, struct fsi, target) > > diff --git a/libpdbg/kernel.c b/libpdbg/kernel.c > index c4637a7..7914e50 100644 > --- a/libpdbg/kernel.c > +++ b/libpdbg/kernel.c > @@ -33,7 +33,6 @@ > #define OPENFSI_LEGACY_PATH "/sys/bus/platform/devices/gpio-fsi/" > #define OPENFSI_PATH "/sys/class/fsi-master/" > > -int fsi_fd; > const char *fsi_base; > > const char *kernel_get_fsi_path(void) > @@ -66,14 +65,14 @@ static int kernel_fsi_getcfam(struct fsi *fsi, uint32_t addr64, uint32_t *value) > int rc; > uint32_t tmp, addr = (addr64 & 0x7ffc00) | ((addr64 & 0x3ff) << 2); > > - rc = lseek(fsi_fd, addr, SEEK_SET); > + rc = lseek(fsi->fd, addr, SEEK_SET); > if (rc < 0) { > rc = errno; > PR_WARNING("seek failed: %s\n", strerror(errno)); > return rc; > } > > - rc = read(fsi_fd, &tmp, 4); > + rc = read(fsi->fd, &tmp, 4); > if (rc < 0) { > rc = errno; > if ((addr64 & 0xfff) != 0xc09) > @@ -93,7 +92,7 @@ static int kernel_fsi_putcfam(struct fsi *fsi, uint32_t addr64, uint32_t data) > int rc; > uint32_t tmp, addr = (addr64 & 0x7ffc00) | ((addr64 & 0x3ff) << 2); > > - rc = lseek(fsi_fd, addr, SEEK_SET); > + rc = lseek(fsi->fd, addr, SEEK_SET); > if (rc < 0) { > rc = errno; > PR_WARNING("seek failed: %s\n", strerror(errno)); > @@ -101,7 +100,7 @@ static int kernel_fsi_putcfam(struct fsi *fsi, uint32_t addr64, uint32_t data) > } > > tmp = htobe32(data); > - rc = write(fsi_fd, &tmp, 4); > + rc = write(fsi->fd, &tmp, 4); > if (rc < 0) { > rc = errno; > PR_ERROR("Failed to write to 0x%08" PRIx32 " (%016" PRIx32 ")\n", addr, addr64); > @@ -111,16 +110,6 @@ static int kernel_fsi_putcfam(struct fsi *fsi, uint32_t addr64, uint32_t data) > return 0; > } > > -#if 0 > -/* TODO: At present we don't have a generic destroy method as there aren't many > - * use cases for it. So for the moment we can just let the OS close the file > - * descriptor on exit. */ > -static void kernel_fsi_destroy(struct pdbg_target *target) > -{ > - close(fsi_fd); > -} > -#endif > - > static void kernel_fsi_scan_devices(void) > { > const char one = '1'; > @@ -154,51 +143,59 @@ static void kernel_fsi_scan_devices(void) > > int kernel_fsi_probe(struct pdbg_target *target) > { > - if (!fsi_fd) { > - int tries = 5; > - int rc; > - const char *kernel_path = kernel_get_fsi_path(); > - char *path; > - > - if (!kernel_path) > - return -1; > - > - rc = asprintf(&path, "%s/fsi0/slave@00:00/raw", kernel_get_fsi_path()); > - if (rc < 0) { > - PR_ERROR("Unable create fsi path\n"); > - return rc; > - } > + struct fsi *fsi = target_to_fsi(target); > + int tries = 5; > + int rc; > + const char *kernel_path = kernel_get_fsi_path(); > + char *path; > > - while (tries) { > - /* Open first raw device */ > - fsi_fd = open(path, O_RDWR | O_SYNC); > - if (fsi_fd >= 0) { > - free(path); > - return 0; > - } > - tries--; > - > - /* Scan */ > - kernel_fsi_scan_devices(); > - sleep(1); > - } > - if (fsi_fd < 0) { > - PR_ERROR("Unable to open %s\n", path); > + if (!kernel_path) > + return -1; > + > + rc = asprintf(&path, "%s/fsi0/slave@00:00/raw", kernel_get_fsi_path()); > + if (rc < 0) { > + PR_ERROR("Unable create fsi path\n"); > + return rc; > + } > + > + while (tries) { > + /* Open first raw device */ > + fsi->fd = open(path, O_RDWR | O_SYNC); > + if (fsi->fd >= 0) { > free(path); > - return -1; > + return 0; Minor nit that doesn't need fixing but IMHO it would probably have been clearer to make this a for loop and break out rather than return. > } > + tries--; > > + /* Scan */ > + kernel_fsi_scan_devices(); > + sleep(1); > + } > + if (fsi->fd < 0) { > + PR_ERROR("Unable to open %s\n", path); > + free(path); > } > > return -1; > } > > +static void kernel_fsi_release(struct pdbg_target *target) > +{ > + struct fsi *fsi = target_to_fsi(target); > + > + if (fsi->fd != -1) { In theory I think generic pdbg logic should prevent this from ever being false as release should only get called on a successful probe. Reviewed-by: Alistair Popple <alistair@popple.id.au> > + close(fsi->fd); > + fsi->fd = -1; > + } > +} > + > static struct fsi kernel_fsi = { > .target = { > .name = "Kernel based FSI master", > .compatible = "ibm,kernel-fsi", > .class = "fsi", > .probe = kernel_fsi_probe, > + .release = kernel_fsi_release, > }, > .read = kernel_fsi_getcfam, > .write = kernel_fsi_putcfam, >
On Mon, 2020-06-15 at 17:22 +1000, Alistair Popple wrote: > On Wednesday, 10 June 2020 3:24:20 PM AEST Amitay Isaacs wrote: > > This allows to have different fd for different fsi targets. Also, > > we > > can close the fd cleanly in release procedure. > > > > Signed-off-by: Amitay Isaacs <amitay@ozlabs.org> > > --- > > libpdbg/hwunit.h | 1 + > > libpdbg/kernel.c | 87 +++++++++++++++++++++++--------------------- > > ---- > > 2 files changed, 43 insertions(+), 45 deletions(-) > > > > diff --git a/libpdbg/hwunit.h b/libpdbg/hwunit.h > > index 61aea0f..e3a8426 100644 > > --- a/libpdbg/hwunit.h > > +++ b/libpdbg/hwunit.h > > @@ -112,6 +112,7 @@ struct fsi { > > int (*read)(struct fsi *, uint32_t, uint32_t *); > > int (*write)(struct fsi *, uint32_t, uint32_t); > > enum chip_type chip_type; > > + int fd; > > }; > > #define target_to_fsi(x) container_of(x, struct fsi, target) > > > > diff --git a/libpdbg/kernel.c b/libpdbg/kernel.c > > index c4637a7..7914e50 100644 > > --- a/libpdbg/kernel.c > > +++ b/libpdbg/kernel.c > > @@ -33,7 +33,6 @@ > > #define OPENFSI_LEGACY_PATH "/sys/bus/platform/devices/gpio-fsi/" > > #define OPENFSI_PATH "/sys/class/fsi-master/" > > > > -int fsi_fd; > > const char *fsi_base; > > > > const char *kernel_get_fsi_path(void) > > @@ -66,14 +65,14 @@ static int kernel_fsi_getcfam(struct fsi *fsi, > > uint32_t > addr64, uint32_t *value) > > int rc; > > uint32_t tmp, addr = (addr64 & 0x7ffc00) | ((addr64 & 0x3ff) << > > 2); > > > > - rc = lseek(fsi_fd, addr, SEEK_SET); > > + rc = lseek(fsi->fd, addr, SEEK_SET); > > if (rc < 0) { > > rc = errno; > > PR_WARNING("seek failed: %s\n", strerror(errno)); > > return rc; > > } > > > > - rc = read(fsi_fd, &tmp, 4); > > + rc = read(fsi->fd, &tmp, 4); > > if (rc < 0) { > > rc = errno; > > if ((addr64 & 0xfff) != 0xc09) > > @@ -93,7 +92,7 @@ static int kernel_fsi_putcfam(struct fsi *fsi, > > uint32_t > addr64, uint32_t data) > > int rc; > > uint32_t tmp, addr = (addr64 & 0x7ffc00) | ((addr64 & 0x3ff) << > > 2); > > > > - rc = lseek(fsi_fd, addr, SEEK_SET); > > + rc = lseek(fsi->fd, addr, SEEK_SET); > > if (rc < 0) { > > rc = errno; > > PR_WARNING("seek failed: %s\n", strerror(errno)); > > @@ -101,7 +100,7 @@ static int kernel_fsi_putcfam(struct fsi *fsi, > > uint32_t > addr64, uint32_t data) > > } > > > > tmp = htobe32(data); > > - rc = write(fsi_fd, &tmp, 4); > > + rc = write(fsi->fd, &tmp, 4); > > if (rc < 0) { > > rc = errno; > > PR_ERROR("Failed to write to 0x%08" PRIx32 " (%016" > > PRIx32 ")\n", > addr, addr64); > > @@ -111,16 +110,6 @@ static int kernel_fsi_putcfam(struct fsi *fsi, > > uint32_t > addr64, uint32_t data) > > return 0; > > } > > > > -#if 0 > > -/* TODO: At present we don't have a generic destroy method as > > there aren't > many > > - * use cases for it. So for the moment we can just let the OS > > close the file > > - * descriptor on exit. */ > > -static void kernel_fsi_destroy(struct pdbg_target *target) > > -{ > > - close(fsi_fd); > > -} > > -#endif > > - > > static void kernel_fsi_scan_devices(void) > > { > > const char one = '1'; > > @@ -154,51 +143,59 @@ static void kernel_fsi_scan_devices(void) > > > > int kernel_fsi_probe(struct pdbg_target *target) > > { > > - if (!fsi_fd) { > > - int tries = 5; > > - int rc; > > - const char *kernel_path = kernel_get_fsi_path(); > > - char *path; > > - > > - if (!kernel_path) > > - return -1; > > - > > - rc = asprintf(&path, "%s/fsi0/slave@00:00/raw", > kernel_get_fsi_path()); > > - if (rc < 0) { > > - PR_ERROR("Unable create fsi path\n"); > > - return rc; > > - } > > + struct fsi *fsi = target_to_fsi(target); > > + int tries = 5; > > + int rc; > > + const char *kernel_path = kernel_get_fsi_path(); > > + char *path; > > > > - while (tries) { > > - /* Open first raw device */ > > - fsi_fd = open(path, O_RDWR | O_SYNC); > > - if (fsi_fd >= 0) { > > - free(path); > > - return 0; > > - } > > - tries--; > > - > > - /* Scan */ > > - kernel_fsi_scan_devices(); > > - sleep(1); > > - } > > - if (fsi_fd < 0) { > > - PR_ERROR("Unable to open %s\n", path); > > + if (!kernel_path) > > + return -1; > > + > > + rc = asprintf(&path, "%s/fsi0/slave@00:00/raw", > > kernel_get_fsi_path()); > > + if (rc < 0) { > > + PR_ERROR("Unable create fsi path\n"); > > + return rc; > > + } > > + > > + while (tries) { > > + /* Open first raw device */ > > + fsi->fd = open(path, O_RDWR | O_SYNC); > > + if (fsi->fd >= 0) { > > free(path); > > - return -1; > > + return 0; > > Minor nit that doesn't need fixing but IMHO it would probably have > been clearer > to make this a for loop and break out rather than return. I can still do that, if that'll make things clearer. > > > } > > + tries--; > > > > + /* Scan */ > > + kernel_fsi_scan_devices(); > > + sleep(1); > > + } > > + if (fsi->fd < 0) { > > + PR_ERROR("Unable to open %s\n", path); > > + free(path); > > } > > > > return -1; > > } > > > > +static void kernel_fsi_release(struct pdbg_target *target) > > +{ > > + struct fsi *fsi = target_to_fsi(target); > > + > > + if (fsi->fd != -1) { > > In theory I think generic pdbg logic should prevent this from ever > being false > as release should only get called on a successful probe. Yes, we can definitely drop the check. I was going nuts trying to figure out what will happen with releasing virtual targets. I am convinced now that each target will be released only once. > > Reviewed-by: Alistair Popple <alistair@popple.id.au> > > > + close(fsi->fd); > > + fsi->fd = -1; > > + } > > +} > > + > > static struct fsi kernel_fsi = { > > .target = { > > .name = "Kernel based FSI master", > > .compatible = "ibm,kernel-fsi", > > .class = "fsi", > > .probe = kernel_fsi_probe, > > + .release = kernel_fsi_release, > > }, > > .read = kernel_fsi_getcfam, > > .write = kernel_fsi_putcfam, > > > > > Amitay.
On Monday, 15 June 2020 5:37:44 PM AEST Amitay Isaacs wrote: > On Mon, 2020-06-15 at 17:22 +1000, Alistair Popple wrote: > > On Wednesday, 10 June 2020 3:24:20 PM AEST Amitay Isaacs wrote: > > > This allows to have different fd for different fsi targets. Also, > > > we > > > can close the fd cleanly in release procedure. > > > > > > Signed-off-by: Amitay Isaacs <amitay@ozlabs.org> > > > --- > > > libpdbg/hwunit.h | 1 + > > > libpdbg/kernel.c | 87 +++++++++++++++++++++++--------------------- > > > ---- > > > 2 files changed, 43 insertions(+), 45 deletions(-) > > > > > > diff --git a/libpdbg/hwunit.h b/libpdbg/hwunit.h > > > index 61aea0f..e3a8426 100644 > > > --- a/libpdbg/hwunit.h > > > +++ b/libpdbg/hwunit.h > > > @@ -112,6 +112,7 @@ struct fsi { > > > int (*read)(struct fsi *, uint32_t, uint32_t *); > > > int (*write)(struct fsi *, uint32_t, uint32_t); > > > enum chip_type chip_type; > > > + int fd; > > > }; > > > #define target_to_fsi(x) container_of(x, struct fsi, target) > > > > > > diff --git a/libpdbg/kernel.c b/libpdbg/kernel.c > > > index c4637a7..7914e50 100644 > > > --- a/libpdbg/kernel.c > > > +++ b/libpdbg/kernel.c > > > @@ -33,7 +33,6 @@ > > > #define OPENFSI_LEGACY_PATH "/sys/bus/platform/devices/gpio-fsi/" > > > #define OPENFSI_PATH "/sys/class/fsi-master/" > > > > > > -int fsi_fd; > > > const char *fsi_base; > > > > > > const char *kernel_get_fsi_path(void) > > > @@ -66,14 +65,14 @@ static int kernel_fsi_getcfam(struct fsi *fsi, > > > uint32_t > > addr64, uint32_t *value) > > > int rc; > > > uint32_t tmp, addr = (addr64 & 0x7ffc00) | ((addr64 & 0x3ff) << > > > 2); > > > > > > - rc = lseek(fsi_fd, addr, SEEK_SET); > > > + rc = lseek(fsi->fd, addr, SEEK_SET); > > > if (rc < 0) { > > > rc = errno; > > > PR_WARNING("seek failed: %s\n", strerror(errno)); > > > return rc; > > > } > > > > > > - rc = read(fsi_fd, &tmp, 4); > > > + rc = read(fsi->fd, &tmp, 4); > > > if (rc < 0) { > > > rc = errno; > > > if ((addr64 & 0xfff) != 0xc09) > > > @@ -93,7 +92,7 @@ static int kernel_fsi_putcfam(struct fsi *fsi, > > > uint32_t > > addr64, uint32_t data) > > > int rc; > > > uint32_t tmp, addr = (addr64 & 0x7ffc00) | ((addr64 & 0x3ff) << > > > 2); > > > > > > - rc = lseek(fsi_fd, addr, SEEK_SET); > > > + rc = lseek(fsi->fd, addr, SEEK_SET); > > > if (rc < 0) { > > > rc = errno; > > > PR_WARNING("seek failed: %s\n", strerror(errno)); > > > @@ -101,7 +100,7 @@ static int kernel_fsi_putcfam(struct fsi *fsi, > > > uint32_t > > addr64, uint32_t data) > > > } > > > > > > tmp = htobe32(data); > > > - rc = write(fsi_fd, &tmp, 4); > > > + rc = write(fsi->fd, &tmp, 4); > > > if (rc < 0) { > > > rc = errno; > > > PR_ERROR("Failed to write to 0x%08" PRIx32 " (%016" > > > PRIx32 ")\n", > > addr, addr64); > > > @@ -111,16 +110,6 @@ static int kernel_fsi_putcfam(struct fsi *fsi, > > > uint32_t > > addr64, uint32_t data) > > > return 0; > > > } > > > > > > -#if 0 > > > -/* TODO: At present we don't have a generic destroy method as > > > there aren't > > many > > > - * use cases for it. So for the moment we can just let the OS > > > close the file > > > - * descriptor on exit. */ > > > -static void kernel_fsi_destroy(struct pdbg_target *target) > > > -{ > > > - close(fsi_fd); > > > -} > > > -#endif > > > - > > > static void kernel_fsi_scan_devices(void) > > > { > > > const char one = '1'; > > > @@ -154,51 +143,59 @@ static void kernel_fsi_scan_devices(void) > > > > > > int kernel_fsi_probe(struct pdbg_target *target) > > > { > > > - if (!fsi_fd) { > > > - int tries = 5; > > > - int rc; > > > - const char *kernel_path = kernel_get_fsi_path(); > > > - char *path; > > > - > > > - if (!kernel_path) > > > - return -1; > > > - > > > - rc = asprintf(&path, "%s/fsi0/slave@00:00/raw", > > kernel_get_fsi_path()); > > > - if (rc < 0) { > > > - PR_ERROR("Unable create fsi path\n"); > > > - return rc; > > > - } > > > + struct fsi *fsi = target_to_fsi(target); > > > + int tries = 5; > > > + int rc; > > > + const char *kernel_path = kernel_get_fsi_path(); > > > + char *path; > > > > > > - while (tries) { > > > - /* Open first raw device */ > > > - fsi_fd = open(path, O_RDWR | O_SYNC); > > > - if (fsi_fd >= 0) { > > > - free(path); > > > - return 0; > > > - } > > > - tries--; > > > - > > > - /* Scan */ > > > - kernel_fsi_scan_devices(); > > > - sleep(1); > > > - } > > > - if (fsi_fd < 0) { > > > - PR_ERROR("Unable to open %s\n", path); > > > + if (!kernel_path) > > > + return -1; > > > + > > > + rc = asprintf(&path, "%s/fsi0/slave@00:00/raw", > > > kernel_get_fsi_path()); > > > + if (rc < 0) { > > > + PR_ERROR("Unable create fsi path\n"); > > > + return rc; > > > + } > > > + > > > + while (tries) { > > > + /* Open first raw device */ > > > + fsi->fd = open(path, O_RDWR | O_SYNC); > > > + if (fsi->fd >= 0) { > > > free(path); > > > - return -1; > > > + return 0; > > > > Minor nit that doesn't need fixing but IMHO it would probably have > > been clearer > > to make this a for loop and break out rather than return. > > I can still do that, if that'll make things clearer. > > > > > > } > > > + tries--; > > > > > > + /* Scan */ > > > + kernel_fsi_scan_devices(); > > > + sleep(1); > > > + } > > > + if (fsi->fd < 0) { > > > + PR_ERROR("Unable to open %s\n", path); > > > + free(path); > > > } > > > > > > return -1; > > > } > > > > > > +static void kernel_fsi_release(struct pdbg_target *target) > > > +{ > > > + struct fsi *fsi = target_to_fsi(target); > > > + > > > + if (fsi->fd != -1) { > > > > In theory I think generic pdbg logic should prevent this from ever > > being false > > as release should only get called on a successful probe. > > Yes, we can definitely drop the check. I was going nuts trying to > figure out what will happen with releasing virtual targets. I am > convinced now that each target will be released only once. Maybe print a warning, but it doesn't really matter because I merged them anyway :-) - Alistair > > > > Reviewed-by: Alistair Popple <alistair@popple.id.au> > > > > > + close(fsi->fd); > > > + fsi->fd = -1; > > > + } > > > +} > > > + > > > static struct fsi kernel_fsi = { > > > .target = { > > > .name = "Kernel based FSI master", > > > .compatible = "ibm,kernel-fsi", > > > .class = "fsi", > > > .probe = kernel_fsi_probe, > > > + .release = kernel_fsi_release, > > > }, > > > .read = kernel_fsi_getcfam, > > > .write = kernel_fsi_putcfam, > > > > > > > > > > > Amitay. >
diff --git a/libpdbg/hwunit.h b/libpdbg/hwunit.h index 61aea0f..e3a8426 100644 --- a/libpdbg/hwunit.h +++ b/libpdbg/hwunit.h @@ -112,6 +112,7 @@ struct fsi { int (*read)(struct fsi *, uint32_t, uint32_t *); int (*write)(struct fsi *, uint32_t, uint32_t); enum chip_type chip_type; + int fd; }; #define target_to_fsi(x) container_of(x, struct fsi, target) diff --git a/libpdbg/kernel.c b/libpdbg/kernel.c index c4637a7..7914e50 100644 --- a/libpdbg/kernel.c +++ b/libpdbg/kernel.c @@ -33,7 +33,6 @@ #define OPENFSI_LEGACY_PATH "/sys/bus/platform/devices/gpio-fsi/" #define OPENFSI_PATH "/sys/class/fsi-master/" -int fsi_fd; const char *fsi_base; const char *kernel_get_fsi_path(void) @@ -66,14 +65,14 @@ static int kernel_fsi_getcfam(struct fsi *fsi, uint32_t addr64, uint32_t *value) int rc; uint32_t tmp, addr = (addr64 & 0x7ffc00) | ((addr64 & 0x3ff) << 2); - rc = lseek(fsi_fd, addr, SEEK_SET); + rc = lseek(fsi->fd, addr, SEEK_SET); if (rc < 0) { rc = errno; PR_WARNING("seek failed: %s\n", strerror(errno)); return rc; } - rc = read(fsi_fd, &tmp, 4); + rc = read(fsi->fd, &tmp, 4); if (rc < 0) { rc = errno; if ((addr64 & 0xfff) != 0xc09) @@ -93,7 +92,7 @@ static int kernel_fsi_putcfam(struct fsi *fsi, uint32_t addr64, uint32_t data) int rc; uint32_t tmp, addr = (addr64 & 0x7ffc00) | ((addr64 & 0x3ff) << 2); - rc = lseek(fsi_fd, addr, SEEK_SET); + rc = lseek(fsi->fd, addr, SEEK_SET); if (rc < 0) { rc = errno; PR_WARNING("seek failed: %s\n", strerror(errno)); @@ -101,7 +100,7 @@ static int kernel_fsi_putcfam(struct fsi *fsi, uint32_t addr64, uint32_t data) } tmp = htobe32(data); - rc = write(fsi_fd, &tmp, 4); + rc = write(fsi->fd, &tmp, 4); if (rc < 0) { rc = errno; PR_ERROR("Failed to write to 0x%08" PRIx32 " (%016" PRIx32 ")\n", addr, addr64); @@ -111,16 +110,6 @@ static int kernel_fsi_putcfam(struct fsi *fsi, uint32_t addr64, uint32_t data) return 0; } -#if 0 -/* TODO: At present we don't have a generic destroy method as there aren't many - * use cases for it. So for the moment we can just let the OS close the file - * descriptor on exit. */ -static void kernel_fsi_destroy(struct pdbg_target *target) -{ - close(fsi_fd); -} -#endif - static void kernel_fsi_scan_devices(void) { const char one = '1'; @@ -154,51 +143,59 @@ static void kernel_fsi_scan_devices(void) int kernel_fsi_probe(struct pdbg_target *target) { - if (!fsi_fd) { - int tries = 5; - int rc; - const char *kernel_path = kernel_get_fsi_path(); - char *path; - - if (!kernel_path) - return -1; - - rc = asprintf(&path, "%s/fsi0/slave@00:00/raw", kernel_get_fsi_path()); - if (rc < 0) { - PR_ERROR("Unable create fsi path\n"); - return rc; - } + struct fsi *fsi = target_to_fsi(target); + int tries = 5; + int rc; + const char *kernel_path = kernel_get_fsi_path(); + char *path; - while (tries) { - /* Open first raw device */ - fsi_fd = open(path, O_RDWR | O_SYNC); - if (fsi_fd >= 0) { - free(path); - return 0; - } - tries--; - - /* Scan */ - kernel_fsi_scan_devices(); - sleep(1); - } - if (fsi_fd < 0) { - PR_ERROR("Unable to open %s\n", path); + if (!kernel_path) + return -1; + + rc = asprintf(&path, "%s/fsi0/slave@00:00/raw", kernel_get_fsi_path()); + if (rc < 0) { + PR_ERROR("Unable create fsi path\n"); + return rc; + } + + while (tries) { + /* Open first raw device */ + fsi->fd = open(path, O_RDWR | O_SYNC); + if (fsi->fd >= 0) { free(path); - return -1; + return 0; } + tries--; + /* Scan */ + kernel_fsi_scan_devices(); + sleep(1); + } + if (fsi->fd < 0) { + PR_ERROR("Unable to open %s\n", path); + free(path); } return -1; } +static void kernel_fsi_release(struct pdbg_target *target) +{ + struct fsi *fsi = target_to_fsi(target); + + if (fsi->fd != -1) { + close(fsi->fd); + fsi->fd = -1; + } +} + static struct fsi kernel_fsi = { .target = { .name = "Kernel based FSI master", .compatible = "ibm,kernel-fsi", .class = "fsi", .probe = kernel_fsi_probe, + .release = kernel_fsi_release, }, .read = kernel_fsi_getcfam, .write = kernel_fsi_putcfam,
This allows to have different fd for different fsi targets. Also, we can close the fd cleanly in release procedure. Signed-off-by: Amitay Isaacs <amitay@ozlabs.org> --- libpdbg/hwunit.h | 1 + libpdbg/kernel.c | 87 +++++++++++++++++++++++------------------------- 2 files changed, 43 insertions(+), 45 deletions(-)