lkml.org 
[lkml]   [2018]   [Sep]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH] drm/amd/display: Change status's type in aux_reply_transaction_data
    On Fri, Sep 21, 2018 at 2:55 PM Nathan Chancellor
    <natechancellor@gmail.com> wrote:
    >
    > Clang warns when one enumerated type is implicitly converted to another.
    >
    > drivers/gpu/drm/amd/amdgpu/../display/dc/dce/dce_aux.c:315:19: warning:
    > implicit conversion from enumeration type 'enum
    > aux_channel_operation_result' to different enumeration type 'enum
    > aux_transaction_reply' [-Wenum-conversion]
    > reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
    > ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    > drivers/gpu/drm/amd/amdgpu/../display/dc/i2caux/dce110/aux_engine_dce110.c:349:19:
    > warning: implicit conversion from enumeration type 'enum
    > aux_channel_operation_result' to different enumeration type 'enum
    > aux_transaction_reply' [-Wenum-conversion]
    > reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
    > ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

    I think the enum is actually wrong here. I think the correct fix would be:

    - reply->status = AUX_CHANNEL_OPERATION_FAILED_HPD_DISCON;
    + reply->status = AUX_TRANSACTION_REPLY_HPD_DISCON;

    The identifiers are so similar, my guess was that it was easy to mix
    them up. This looks like an actual bug to me, since the identifiers
    have different values between the 2 different enums.

    >
    > Instead of implicitly or explicitly converting between types, just
    > change status to type uint8_t (since its max size is 255) which avoids
    > this construct altogether.
    >
    > Reported-by: Nick Desaulniers <ndesaulniers@google.com>
    > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
    > ---
    > drivers/gpu/drm/amd/display/dc/dc_ddc_types.h | 2 +-
    > 1 file changed, 1 insertion(+), 1 deletion(-)
    >
    > diff --git a/drivers/gpu/drm/amd/display/dc/dc_ddc_types.h b/drivers/gpu/drm/amd/display/dc/dc_ddc_types.h
    > index 05c8c31d8b31..97e1d4d19263 100644
    > --- a/drivers/gpu/drm/amd/display/dc/dc_ddc_types.h
    > +++ b/drivers/gpu/drm/amd/display/dc/dc_ddc_types.h
    > @@ -79,7 +79,7 @@ enum aux_transaction_reply {
    > };
    >
    > struct aux_reply_transaction_data {
    > - enum aux_transaction_reply status;
    > + uint8_t status;
    > uint32_t length;
    > uint8_t *data;
    > };
    > --
    > 2.19.0
    >


    --
    Thanks,
    ~Nick Desaulniers

    \
     
     \ /
      Last update: 2018-09-25 00:07    [W:2.821 / U:0.504 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site