[Pdbg] [PATCH v2] kernel: Use FSI master 'class' path

Amitay Isaacs amitay at ozlabs.org
Thu Sep 26 16:20:31 AEST 2019


On Thu, 2019-09-26 at 15:41 +0930, 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 at jms.id.au>
> ---
> v2:
>  - Test for new path first so that code is being tested/used
>  - Print a debug message when no devices found
> ---
>  libpdbg/dtb.c     | 14 ++++++++--
>  libpdbg/kernel.c  | 67 ++++++++++++++++++++++++++++++++++++++++-----
> --
>  libpdbg/libpdbg.h |  2 ++
>  3 files changed, 71 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 at 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..6d88298abfa7 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,36 @@
>  #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 at 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_PATH, F_OK);
> +	if (rc == 0) {
> +		fsi_base = OPENFSI_PATH;
> +		return fsi_base;
> +	}
> +
> +	rc = access(OPENFSI_LEGACY_PATH, F_OK);
> +	if (rc == 0) {
> +		fsi_base = OPENFSI_LEGACY_PATH;
> +		return fsi_base;
> +	}
> +
> +	/* This is an error, but callers use this function when probing
> */
> +	PR_DEBUG("Failed to find kernel FSI path\n");
> +
> +	return NULL;
> +}
>  
>  static int kernel_fsi_getcfam(struct fsi *fsi, uint32_t addr64,
> uint32_t *value)
>  {
> @@ -42,7 +69,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));

I have a patch that fixes all the log messages which have missing
newlines.  Since you are touching most of the log messages in this
file, do you want to add the missing newlines?


>  		return rc;
>  	}
>  
> @@ -69,7 +96,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 +124,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 +152,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 +175,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
> 

Amitay.
-- 

Whatever does not kill you makes you strong.  - Friedrich Nietsche



More information about the Pdbg mailing list