| Message ID | 20190923074638.15866-1-joel@jms.id.au |
|---|---|
| State | Superseded |
| Headers | show |
| Series | kernel: Use FSI master 'class' path | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch master (8a10a05c89db666bf98734139334166da7c370a4) |
| snowpatch_ozlabs/apply_patch | fail | Failed to apply to any branch |
On Mon, 23 Sep 2019, at 17:16, Joel Stanley wrote: > The paths to sysfs entries depend on the master driver used. This means > pdbg needs to use a different path when running against the hardware FSI > master present in the ast2600. > > Instead of adding to an ever growing list of paths to check, the kernel > has created a 'fsi-master' class that any driver can add itself to. On a > kernel runnign this code, userspace only needs to check under > /sys/class/fsi-master for any platform. > > This patch adds support for the class based path from a common accessor > in kernel.c. > > It retains support for the existing gpio-fsi master. At some point in > the distant future the gpio-fsi code could be deleted. > > Signed-off-by: Joel Stanley <joel@jms.id.au> > --- > libpdbg/dtb.c | 14 +++++++++-- > libpdbg/kernel.c | 64 +++++++++++++++++++++++++++++++++++++++-------- > libpdbg/libpdbg.h | 2 ++ > 3 files changed, 68 insertions(+), 12 deletions(-) > > diff --git a/libpdbg/dtb.c b/libpdbg/dtb.c > index 6c4beeda4fe6..2aea625feaf1 100644 > --- a/libpdbg/dtb.c > +++ b/libpdbg/dtb.c > @@ -13,6 +13,7 @@ > * See the License for the specific language governing permissions and > * limitations under the License. > */ > +#define _GNU_SOURCE > #include <stdio.h> > #include <stdlib.h> > #include <stdbool.h> > @@ -68,7 +69,7 @@ static enum pdbg_backend default_backend(void) > if (rc == 0) /* AMI BMC */ > return PDBG_BACKEND_I2C; > > - rc = access(OPENFSI_BMC, F_OK); > + rc = access(kernel_get_fsi_path(), F_OK); > if (rc == 0) /* Kernel interface. OpenBMC */ > return PDBG_BACKEND_KERNEL; > > @@ -134,7 +135,16 @@ static void *bmc_target(void) > > if (!pdbg_backend_option) { > /* Try and determine the correct device type */ > - cfam_id_file = fopen(FSI_CFAM_ID, "r"); > + char *path; > + > + rc = asprintf(&path, "%s/fsi0/slave@00:00/cfam_id", kernel_get_fsi_path()); > + if (rc < 0) { > + pdbg_log(PDBG_ERROR, "Unable create fsi path"); > + return NULL; > + } > + > + cfam_id_file = fopen(path, "r"); > + free(path); > if (!cfam_id_file) { > pdbg_log(PDBG_ERROR, "Unabled to open CFAM ID file\n"); > return NULL; > diff --git a/libpdbg/kernel.c b/libpdbg/kernel.c > index 4cc0334d7eb3..4a57fc249a04 100644 > --- a/libpdbg/kernel.c > +++ b/libpdbg/kernel.c > @@ -13,6 +13,7 @@ > * See the License for the specific language governing permissions and > * limitations under the License. > */ > +#define _GNU_SOURCE > #include <stdint.h> > #include <stdio.h> > #include <stdlib.h> > @@ -29,10 +30,33 @@ > #include "operations.h" > #include "hwunit.h" > > -#define FSI_SCAN_PATH "/sys/bus/platform/devices/gpio-fsi/fsi0/rescan" > -#define FSI_CFAM_PATH "/sys/devices/platform/gpio-fsi/fsi0/slave@00:00/raw" > +#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) > +{ > + int rc; > + > + if (fsi_base) > + return fsi_base; > + > + rc = access(OPENFSI_LEGACY_PATH, F_OK); > + if (rc == 0) { > + fsi_base = OPENFSI_LEGACY_PATH; > + return fsi_base; > + } I think we should try the new path first and fall back to the legacy path. That way we're exercising the new path on supported kernels now, not when the legacy path goes away. > + > + rc = access(OPENFSI_PATH, F_OK); > + if (rc == 0) { > + fsi_base = OPENFSI_PATH; > + return fsi_base; > + } > + > + return NULL; > +} > > static int kernel_fsi_getcfam(struct fsi *fsi, uint32_t addr64, > uint32_t *value) > { > @@ -42,7 +66,7 @@ static int kernel_fsi_getcfam(struct fsi *fsi, > uint32_t addr64, uint32_t *value) > rc = lseek(fsi_fd, addr, SEEK_SET); > if (rc < 0) { > rc = errno; > - PR_WARNING("Failed to seek %s", FSI_CFAM_PATH); > + PR_WARNING("seek failed: %s", strerror(errno)); > return rc; > } > > @@ -69,7 +93,7 @@ static int kernel_fsi_putcfam(struct fsi *fsi, > uint32_t addr64, uint32_t data) > rc = lseek(fsi_fd, addr, SEEK_SET); > if (rc < 0) { > rc = errno; > - PR_WARNING("Failed to seek %s", FSI_CFAM_PATH); > + PR_WARNING("seek failed: %s", strerror(errno)); > return rc; > } > > @@ -97,18 +121,27 @@ static void kernel_fsi_destroy(struct pdbg_target *target) > static void kernel_fsi_scan_devices(void) > { > const char one = '1'; > + char *path; > int rc, fd; > > - fd = open(FSI_SCAN_PATH, O_WRONLY | O_SYNC); > + rc = asprintf(&path, "%s/fsi0/rescan", kernel_get_fsi_path()); > + if (rc < 0) { > + PR_ERROR("Unable create fsi path"); > + return; > + } > + > + fd = open(path, O_WRONLY | O_SYNC); > if (fd < 0) { > - PR_ERROR("Unable to open %s", FSI_SCAN_PATH); > + PR_ERROR("Unable to open %s", path); > + free(path); > return; > } > > rc = write(fd, &one, sizeof(one)); > if (rc < 0) > - PR_ERROR("Unable to write to %s", FSI_SCAN_PATH); > + PR_ERROR("Unable to write to %s", path); > > + free(path); > close(fd); > } > > @@ -116,12 +149,22 @@ int kernel_fsi_probe(struct pdbg_target *target) > { > if (!fsi_fd) { > int tries = 5; > + int rc; > + char *path; > + > + rc = asprintf(&path, "%s/fsi0/rescan", kernel_get_fsi_path()); > + if (rc < 0) { > + PR_ERROR("Unable create fsi path"); > + return rc; > + } > > while (tries) { > /* Open first raw device */ > - fsi_fd = open(FSI_CFAM_PATH, O_RDWR | O_SYNC); > - if (fsi_fd >= 0) > + fsi_fd = open(path, O_RDWR | O_SYNC); > + if (fsi_fd >= 0) { > + free(path); > return 0; > + } > tries--; > > /* Scan */ > @@ -129,7 +172,8 @@ int kernel_fsi_probe(struct pdbg_target *target) > sleep(1); > } > if (fsi_fd < 0) { > - PR_ERROR("Unable to open %s", FSI_CFAM_PATH); > + PR_ERROR("Unable to open %s", path); > + free(path); > return -1; > } > > diff --git a/libpdbg/libpdbg.h b/libpdbg/libpdbg.h > index 235ff85abdc6..468553bbf4b5 100644 > --- a/libpdbg/libpdbg.h > +++ b/libpdbg/libpdbg.h > @@ -121,6 +121,8 @@ void pdbg_target_priv_set(struct pdbg_target > *target, void *priv); > struct pdbg_target *pdbg_target_root(void); > bool pdbg_target_compatible(struct pdbg_target *target, const char > *compatible); > > +const char *kernel_get_fsi_path(void); > + > /* Translate an address offset for a target to absolute address in > address > * space of a "base" target. */ > struct pdbg_target *pdbg_address_absolute(struct pdbg_target *target, > uint64_t *addr); > -- > 2.23.0 > > -- > Pdbg mailing list > Pdbg@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/pdbg >
On Wed, 25 Sep 2019 at 06:23, Andrew Jeffery <andrew@aj.id.au> wrote: > > + rc = access(OPENFSI_LEGACY_PATH, F_OK); > > + if (rc == 0) { > > + fsi_base = OPENFSI_LEGACY_PATH; > > + return fsi_base; > > + } > > I think we should try the new path first and fall back to the legacy path. > That way we're exercising the new path on supported kernels now, not > when the legacy path goes away. Good idea. I'll send a v2. Cheers, Joel
diff --git a/libpdbg/dtb.c b/libpdbg/dtb.c index 6c4beeda4fe6..2aea625feaf1 100644 --- a/libpdbg/dtb.c +++ b/libpdbg/dtb.c @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +#define _GNU_SOURCE #include <stdio.h> #include <stdlib.h> #include <stdbool.h> @@ -68,7 +69,7 @@ static enum pdbg_backend default_backend(void) if (rc == 0) /* AMI BMC */ return PDBG_BACKEND_I2C; - rc = access(OPENFSI_BMC, F_OK); + rc = access(kernel_get_fsi_path(), F_OK); if (rc == 0) /* Kernel interface. OpenBMC */ return PDBG_BACKEND_KERNEL; @@ -134,7 +135,16 @@ static void *bmc_target(void) if (!pdbg_backend_option) { /* Try and determine the correct device type */ - cfam_id_file = fopen(FSI_CFAM_ID, "r"); + char *path; + + rc = asprintf(&path, "%s/fsi0/slave@00:00/cfam_id", kernel_get_fsi_path()); + if (rc < 0) { + pdbg_log(PDBG_ERROR, "Unable create fsi path"); + return NULL; + } + + cfam_id_file = fopen(path, "r"); + free(path); if (!cfam_id_file) { pdbg_log(PDBG_ERROR, "Unabled to open CFAM ID file\n"); return NULL; diff --git a/libpdbg/kernel.c b/libpdbg/kernel.c index 4cc0334d7eb3..4a57fc249a04 100644 --- a/libpdbg/kernel.c +++ b/libpdbg/kernel.c @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +#define _GNU_SOURCE #include <stdint.h> #include <stdio.h> #include <stdlib.h> @@ -29,10 +30,33 @@ #include "operations.h" #include "hwunit.h" -#define FSI_SCAN_PATH "/sys/bus/platform/devices/gpio-fsi/fsi0/rescan" -#define FSI_CFAM_PATH "/sys/devices/platform/gpio-fsi/fsi0/slave@00:00/raw" +#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) +{ + int rc; + + if (fsi_base) + return fsi_base; + + rc = access(OPENFSI_LEGACY_PATH, F_OK); + if (rc == 0) { + fsi_base = OPENFSI_LEGACY_PATH; + return fsi_base; + } + + rc = access(OPENFSI_PATH, F_OK); + if (rc == 0) { + fsi_base = OPENFSI_PATH; + return fsi_base; + } + + return NULL; +} static int kernel_fsi_getcfam(struct fsi *fsi, uint32_t addr64, uint32_t *value) { @@ -42,7 +66,7 @@ static int kernel_fsi_getcfam(struct fsi *fsi, uint32_t addr64, uint32_t *value) rc = lseek(fsi_fd, addr, SEEK_SET); if (rc < 0) { rc = errno; - PR_WARNING("Failed to seek %s", FSI_CFAM_PATH); + PR_WARNING("seek failed: %s", strerror(errno)); return rc; } @@ -69,7 +93,7 @@ static int kernel_fsi_putcfam(struct fsi *fsi, uint32_t addr64, uint32_t data) rc = lseek(fsi_fd, addr, SEEK_SET); if (rc < 0) { rc = errno; - PR_WARNING("Failed to seek %s", FSI_CFAM_PATH); + PR_WARNING("seek failed: %s", strerror(errno)); return rc; } @@ -97,18 +121,27 @@ static void kernel_fsi_destroy(struct pdbg_target *target) static void kernel_fsi_scan_devices(void) { const char one = '1'; + char *path; int rc, fd; - fd = open(FSI_SCAN_PATH, O_WRONLY | O_SYNC); + rc = asprintf(&path, "%s/fsi0/rescan", kernel_get_fsi_path()); + if (rc < 0) { + PR_ERROR("Unable create fsi path"); + return; + } + + fd = open(path, O_WRONLY | O_SYNC); if (fd < 0) { - PR_ERROR("Unable to open %s", FSI_SCAN_PATH); + PR_ERROR("Unable to open %s", path); + free(path); return; } rc = write(fd, &one, sizeof(one)); if (rc < 0) - PR_ERROR("Unable to write to %s", FSI_SCAN_PATH); + PR_ERROR("Unable to write to %s", path); + free(path); close(fd); } @@ -116,12 +149,22 @@ int kernel_fsi_probe(struct pdbg_target *target) { if (!fsi_fd) { int tries = 5; + int rc; + char *path; + + rc = asprintf(&path, "%s/fsi0/rescan", kernel_get_fsi_path()); + if (rc < 0) { + PR_ERROR("Unable create fsi path"); + return rc; + } while (tries) { /* Open first raw device */ - fsi_fd = open(FSI_CFAM_PATH, O_RDWR | O_SYNC); - if (fsi_fd >= 0) + fsi_fd = open(path, O_RDWR | O_SYNC); + if (fsi_fd >= 0) { + free(path); return 0; + } tries--; /* Scan */ @@ -129,7 +172,8 @@ int kernel_fsi_probe(struct pdbg_target *target) sleep(1); } if (fsi_fd < 0) { - PR_ERROR("Unable to open %s", FSI_CFAM_PATH); + PR_ERROR("Unable to open %s", path); + free(path); return -1; } diff --git a/libpdbg/libpdbg.h b/libpdbg/libpdbg.h index 235ff85abdc6..468553bbf4b5 100644 --- a/libpdbg/libpdbg.h +++ b/libpdbg/libpdbg.h @@ -121,6 +121,8 @@ void pdbg_target_priv_set(struct pdbg_target *target, void *priv); struct pdbg_target *pdbg_target_root(void); bool pdbg_target_compatible(struct pdbg_target *target, const char *compatible); +const char *kernel_get_fsi_path(void); + /* Translate an address offset for a target to absolute address in address * space of a "base" target. */ struct pdbg_target *pdbg_address_absolute(struct pdbg_target *target, uint64_t *addr);
The paths to sysfs entries depend on the master driver used. This means pdbg needs to use a different path when running against the hardware FSI master present in the ast2600. Instead of adding to an ever growing list of paths to check, the kernel has created a 'fsi-master' class that any driver can add itself to. On a kernel runnign this code, userspace only needs to check under /sys/class/fsi-master for any platform. This patch adds support for the class based path from a common accessor in kernel.c. It retains support for the existing gpio-fsi master. At some point in the distant future the gpio-fsi code could be deleted. Signed-off-by: Joel Stanley <joel@jms.id.au> --- libpdbg/dtb.c | 14 +++++++++-- libpdbg/kernel.c | 64 +++++++++++++++++++++++++++++++++++++++-------- libpdbg/libpdbg.h | 2 ++ 3 files changed, 68 insertions(+), 12 deletions(-)