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

[Alistair Popple]

On Friday, 27 September 2019 5:02:40 PM AEST Andrew Jeffery wrote:
> 
> On Thu, 26 Sep 2019, at 16:31, 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>
> > ---
> > v3:
> >  - fix probe bug
> >  - add newlines to all of the print statements
> >  - no need to call access() at the in dtb.c, just check if we have a
> >  valid path
> > v2:
> >  - Test for new path first so that code is being tested/used
> >  - Print a debug message when no devices found
> > ---
> >  libpdbg/dtb.c     | 15 ++++++++--
> >  libpdbg/kernel.c  | 71 +++++++++++++++++++++++++++++++++++++++--------
> >  libpdbg/libpdbg.h |  2 ++
> >  3 files changed, 73 insertions(+), 15 deletions(-)
> > 
> > diff --git a/libpdbg/dtb.c b/libpdbg/dtb.c
> > index 6c4beeda4fe6..53e9b7351e1b 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,8 +69,7 @@ static enum pdbg_backend default_backend(void)
> >  	if (rc == 0) /* AMI BMC */
> >  		return PDBG_BACKEND_I2C;
> >  
> > -	rc = access(OPENFSI_BMC, F_OK);
> > -	if (rc == 0) /* Kernel interface. OpenBMC */
> > +	if (kernel_get_fsi_path() != NULL) /* Kernel interface. OpenBMC */
> >  		return PDBG_BACKEND_KERNEL;
> >  
> >  	return PDBG_BACKEND_FAKE;
> > @@ -134,7 +134,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());
> 
> It looks like we could still get NULL from kernel_get_fsi_path() here if the 
user has
> specified the kernel backend on the commandline. We're fine if the backend is
> autodetected as the selected backend is predicated on kernel_get_fsi_path() 
not
> returning NULL in the previous hunk.
> 
> Maybe the user gets what they deserve in this case, but it probably isn't 
the
> mechanism we should use for erroring out.

Right. A segfault is always a sub-optimal error. Thanks for the review, I can 
do a simple fix  when I merge it.

- Alistair

> Andrew
>