On Mon, 2013-04-22 at 10:18 -0400, Alex Deucher wrote: 
> On Mon, Apr 22, 2013 at 10:08 AM, Dan Carpenter
> <dan.carpenter at oracle.com> wrote:
> > On Mon, Apr 22, 2013 at 10:03:13AM -0400, alexdeucher at gmail.com wrote:
> >> From: Alex Deucher <alexander.deucher at amd.com>
> >>
> >> Reported-by: Dan Carpenter <dan.carpenter at oracle.com>
> >> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
> >> ---
> >>  drivers/gpu/drm/radeon/atombios.h        |    2 ++
> >>  drivers/gpu/drm/radeon/radeon_atombios.c |    6 ++----
> >>  2 files changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/radeon/atombios.h 
> >> b/drivers/gpu/drm/radeon/atombios.h
> >> index 4b04ba3..de678dd 100644
> >> --- a/drivers/gpu/drm/radeon/atombios.h
> >> +++ b/drivers/gpu/drm/radeon/atombios.h
[...] 
> >> @@ -491,6 +492,7 @@ typedef struct _COMPUTE_MEMORY_ENGINE_PLL_PARAMETERS_V5
> >>    {
> >>      ATOM_COMPUTE_CLOCK_FREQ  ulClock;         //Input Parameter
> >>      ATOM_S_MPLL_FB_DIVIDER   ulFbDiv;         //Output Parameter
> >> +    ULONG ulClockFbDiv;
> >>    };
> >>    UCHAR   ucRefDiv;                           //Output Parameter
> >>    UCHAR   ucPostDiv;                          //Output Parameter

This may just be a nitpick, but the location and name of the new union
member is slightly confusing: It suggests that it's some kind of
combination of the ulClock and ulFbDiv members, when it's just an
alternative representation of ulClock. I'd suggest moving up the new
member, clarifying its name and/or adding a comment explaining what it
is for. 
Looks good to me other than that.


-- 
Earthling Michel D?nzer           |                   http://www.amd.com
Libre software enthusiast         |          Debian, X and DRI developer

Reply via email to