On Thu, Apr 26, 2007 at 04:55:14PM +1000, Dave Airlie wrote:
> Hi,
>
> The patch is too big to fit on the list and I've no idea how we could
> break it down further, it just happens to be a lot of new code..
>
>
> http://people.freedesktop.org/~airlied/ttm/0001-drm-Implement-TTM-Memory-manager-core-functionality.txt
>
> The patch header and diffstat are below,
A few easy and simple comments based on looking at this for 5 minutes:
- drop the typedefs. Yeah, they might be a drm thing, but we don't
need them here.
- the comment style for the functions is "odd" and not in kerneldoc
form, but something else. Either use kerneldoc or nothing, don't
invent something new please.
- what's with the /proc interface? Don't add new proc code for
non-process related things. This should all go into sysfs
somewhere. And yes, I know /proc/dri/ is there today, but don't add
new stuff please.
- struct drm_bo_arg can't use an int or unsigned, as it crosses the
userspace/kernelspace boundry, use the proper types for all values
in those types of structures (__u32, etc.)
- there doesn't seem to be any validity checking for the arguments
passed into this new ioctl. Possibly that's just the way the rest
of the dri interface is, which is scary, but with the memory stuff,
you really should check things properly...
that's enough to start with :)
thanks,
greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
