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

[Amitay Isaacs]

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.

> 
> - 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.
-- 

Don't wait for your ship to come in. Row out to meet it.