[Pixman] [PATCH] MIPS: DSPr2: Basic infrastructure for MIPS architecture

[Søren Sandmann]

Hi,

Thanks for the patch. The main comment I have is about being precise
that this is about DSPr2 and not r1. This has some consequences:

- I think a consistent string "dspr2" should be used to refer to this
  functionality. Ie., the files should be called pixman-mips-dspr2.[ch],
  the functions and variables should have "dspr2" in their names, and
  not "dsp" etc. The SSE2 implementation used to use "sse" and "sse2"
  interchangably and it resulted in total confusion.

- The runtime CPU detection seems to check whether the "dsp" is found in
  /proc/cpuinfo. However, from reading the kernel source, it looks like
  "dsp" only indicates that rev. 1 of the DSP ASE is available.

  I also think the search function may be buggy. Consider what happens
  when given the string "ddsp"

> +  while (EOF != (k = fgetc(f))) {
> +    if (k == *what) {
> +      ++what;
> +      while ((*what != '\0') && (*what == fgetc(f))) {
> +        ++what;
> +      }
> +      if (*what == '\0') {
> +        fclose(f);
> +        return TRUE;
> +      } else {
> +        what = search_string;
> +      }
> +    }
> +  }

  Consider what it does when given the string "ddsp". The "k == *what
  will be true, and so the second fgetc(f) will eat the second
  'd'. Since *what is not \0 at this point, it will be reset to "dsp",
  but the file is now at the "sp" point. Ie., it will miss the "dsp"
  string in "ddsp".

  Personally, I think I'd do it as just a loop using fgets() into some
  reasonable-sized buffer, and then checking each line with strstr().

  But the bigger problem is that the kernel apparently doesn't offer any
  way to detect dspr2 that I can tell.

Also, there are some coding style issues. Please see the file
CODING_STYLE. In particular,

- Use /* ... */ for comments, not //
- Don't use braces with single-line statements
- Put braces on their own line
- Format functions with their return type on its own line

Finally, feel free to send all the patches that you think are ready to
be reviewed. There is no need to wait for each one to be reviewed. Note
that

    git send-email <last commit-id before your patches> 

is convenient for this.


Søren