[PATCH] drm/radeon: fix endian bugs in radeon_atom_get_clock_dividers()

Michel Dänzer michel at daenzer.net
Mon Apr 22 08:29:48 PDT 2013


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


More information about the dri-devel mailing list