[Pdbg] [PATCH 1/3] istep: Add checking for istep major/minor numbers

[Alistair Popple]

On Thursday, 11 July 2019 3:46:44 PM AEST Amitay Isaacs wrote:
> On Thu, 2019-07-11 at 15:39 +1000, Alistair Popple wrote:
> > Part of me feels like one day we will need a way of over-riding this
> > (say when
> > the SBE decides that istep 5.3 is a thing). Can we just print a
> > warning that
> > things may fail instead? From memory the SBE will just respond with
> > an error
> > if the wrong numbers are specified anyway, so it shouldn't cause a
> > big issue.
> > 
> > I assume there is no way of probing the SBE to find out what isteps
> > it
> > supports? Do you think it would make sense to just keep incrementing
> > until we
> > get an invalid argument failure from the SBE?
> 
> No.  The whole SBE chip-op istep interface sucks.  We can't do anything
> better.  I will check if they reply with specific error code in case
> it's an invalid major/minor number.  If not, then there is no way to
> distinguish between failed istep and invalid istep.

Ok. Maybe we can get that fixed. At least it can't hurt to ask (or send 
patches) :-)

- Alistair
 
> > - Alistair
> > 
> > On Thursday, 4 July 2019 6:11:31 PM AEST Amitay Isaacs wrote:
> > > Signed-off-by: Amitay Isaacs <amitay at ozlabs.org>
> > > ---
> > > 
> > >  src/istep.c | 30 ++++++++++++++++++++++++++----
> > >  1 file changed, 26 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/src/istep.c b/src/istep.c
> > > index 08a6215..a9acbe7 100644
> > > --- a/src/istep.c
> > > +++ b/src/istep.c
> > > @@ -20,18 +20,40 @@
> > > 
> > >  #include "optcmd.h"
> > >  #include "path.h"
> > > 
> > > +struct istep_data {
> > > +	int major;
> > > +	int minor_first;
> > > +	int minor_last;
> > > +} istep_data[] = {
> > > +	{ 2, 2, 17 },
> > > +	{ 3, 1, 22 },
> > > +	{ 4, 1, 34 },
> > > +	{ 5, 1,  2 },
> > > +	{ 0, 0, 0  },
> > > +};
> > > +
> > > 
> > >  static int istep(uint32_t major, uint32_t minor)
> > >  {
> > >  
> > >  	struct pdbg_target *target;
> > > 
> > > -	int count = 0;
> > > +	int count = 0, i;
> > > 
> > >  	if (major < 2 || major > 5) {
> > >  	
> > >  		fprintf(stderr, "Istep major should be 2 to 5\n");
> > >  		return 0;
> > >  	
> > >  	}
> > > 
> > > -	if (major == 2 && minor == 1) {
> > > -		fprintf(stderr, "Istep 2 minor should be > 1\n");
> > > -		return 0;
> > > +
> > > +	for (i=0; istep_data[i].major != 0; i++) {
> > > +		if (istep_data[i].major != major)
> > > +			continue;
> > > +
> > > +		if (minor < istep_data[i].minor_first ||
> > > +		    minor > istep_data[i].minor_last) {
> > > +			fprintf(stderr, "Istep %d minor should be %d to
> > > %d\n",
> > > +				major,
> > > +				istep_data[i].minor_first,
> > > +				istep_data[i].minor_last);
> > > +			return 0;
> > > +		}
> > > 
> > >  	}
> > >  	
> > >  	for_each_path_target_class("sbefifo", target) {
> 
> Amitay.