diff mbox series

drm/mediatek: cleanup coding style in mediatek a bit

Message ID 20200427075238.2828-1-bernard@vivo.com (mailing list archive)
State New, archived
Headers show
Series drm/mediatek: cleanup coding style in mediatek a bit | expand

Commit Message

Bernard Zhao April 27, 2020, 7:52 a.m. UTC
This code change is to make code bit more readable.
Optimise array size align to HDMI macro define.
Add check if len is overange.

Signed-off-by: Bernard Zhao <bernard@vivo.com>
---
 drivers/gpu/drm/mediatek/mtk_hdmi.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

Comments

Chun-Kuang Hu April 29, 2020, 2:22 p.m. UTC | #1
Hi, Bernard:

Bernard Zhao <bernard@vivo.com> 於 2020年4月27日 週一 下午3:53寫道:
>
> This code change is to make code bit more readable.
> Optimise array size align to HDMI macro define.
> Add check if len is overange.

One patch should just do one thing, but this do three things.
So break this into three patches.

Regards,
Chun-Kuang.

>
> Signed-off-by: Bernard Zhao <bernard@vivo.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_hdmi.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> index ff43a3d80410..40fb5154ed5d 100644
> --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> @@ -311,15 +311,15 @@ static void mtk_hdmi_hw_send_info_frame(struct mtk_hdmi *hdmi, u8 *buffer,
>         u8 checksum;
>         int ctrl_frame_en = 0;
>
> -       frame_type = *buffer;
> -       buffer += 1;
> -       frame_ver = *buffer;
> -       buffer += 1;
> -       frame_len = *buffer;
> -       buffer += 1;
> -       checksum = *buffer;
> -       buffer += 1;
> +       frame_type = *buffer++;
> +       frame_ver = *buffer++;
> +       frame_len = *buffer++;
> +       checksum = *buffer++;
>         frame_data = buffer;
> +       if ((frame_len + HDMI_INFOFRAME_HEADER_SIZE) > len) {
> +               dev_err(hdmi->dev, "Wrong frame len: %d\n", frame_len;
> +               return;
> +       }
>
>         dev_dbg(hdmi->dev,
>                 "frame_type:0x%x,frame_ver:0x%x,frame_len:0x%x,checksum:0x%x\n",
> @@ -982,7 +982,7 @@ static int mtk_hdmi_setup_avi_infoframe(struct mtk_hdmi *hdmi,
>                                         struct drm_display_mode *mode)
>  {
>         struct hdmi_avi_infoframe frame;
> -       u8 buffer[17];
> +       u8 buffer[HDMI_INFOFRAME_HEADER_SIZE + HDMI_AVI_INFOFRAME_SIZE];
>         ssize_t err;
>
>         err = drm_hdmi_avi_infoframe_from_display_mode(&frame,
> @@ -1008,7 +1008,7 @@ static int mtk_hdmi_setup_spd_infoframe(struct mtk_hdmi *hdmi,
>                                         const char *product)
>  {
>         struct hdmi_spd_infoframe frame;
> -       u8 buffer[29];
> +       u8 buffer[HDMI_INFOFRAME_HEADER_SIZE + HDMI_SPD_INFOFRAME_SIZE];
>         ssize_t err;
>
>         err = hdmi_spd_infoframe_init(&frame, vendor, product);
> @@ -1031,7 +1031,7 @@ static int mtk_hdmi_setup_spd_infoframe(struct mtk_hdmi *hdmi,
>  static int mtk_hdmi_setup_audio_infoframe(struct mtk_hdmi *hdmi)
>  {
>         struct hdmi_audio_infoframe frame;
> -       u8 buffer[14];
> +       u8 buffer[HDMI_INFOFRAME_HEADER_SIZE + HDMI_AUDIO_INFOFRAME_SIZE];
>         ssize_t err;
>
>         err = hdmi_audio_infoframe_init(&frame);
> --
> 2.26.2
>
>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
Bernard Zhao April 30, 2020, 6:31 a.m. UTC | #2
发件人:Chun-Kuang Hu <chunkuang.hu@kernel.org>
发送日期:2020-04-29 22:22:50
收件人:Bernard Zhao <bernard@vivo.com>
抄送人:Chun-Kuang Hu <chunkuang.hu@kernel.org>,Philipp Zabel <p.zabel@pengutronix.de>,David Airlie <airlied@linux.ie>,Daniel Vetter <daniel@ffwll.ch>,Matthias Brugger <matthias.bgg@gmail.com>,DRI Development <dri-devel@lists.freedesktop.org>,Linux ARM <linux-arm-kernel@lists.infradead.org>,"moderated list:ARM/Mediatek SoC support" <linux-mediatek@lists.infradead.org>,linux-kernel <linux-kernel@vger.kernel.org>,opensource.kernel@vivo.com
主题:Re: [PATCH] drm/mediatek: cleanup coding style in mediatek a bit>Hi, Bernard:
>
>Bernard Zhao <bernard@vivo.com> 於 2020年4月27日 週一 下午3:53寫道:
>>
>> This code change is to make code bit more readable.
>> Optimise array size align to HDMI macro define.
>> Add check if len is overange.
>
>One patch should just do one thing, but this do three things.
>So break this into three patches.
>
>Regards,
>Chun-Kuang.

Hi
This optimization is mainly to make the code a bit readable.
These modifications are related, main in several related function calls in the same file.
I was a bit confused that if it is really necessary to change to three separate patch submissions?

Regards
Bernard

>>
>> Signed-off-by: Bernard Zhao <bernard@vivo.com>
>> ---
>>  drivers/gpu/drm/mediatek/mtk_hdmi.c | 22 +++++++++++-----------
>>  1 file changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
>> index ff43a3d80410..40fb5154ed5d 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
>> @@ -311,15 +311,15 @@ static void mtk_hdmi_hw_send_info_frame(struct mtk_hdmi *hdmi, u8 *buffer,
>>         u8 checksum;
>>         int ctrl_frame_en = 0;
>>
>> -       frame_type = *buffer;
>> -       buffer += 1;
>> -       frame_ver = *buffer;
>> -       buffer += 1;
>> -       frame_len = *buffer;
>> -       buffer += 1;
>> -       checksum = *buffer;
>> -       buffer += 1;
>> +       frame_type = *buffer++;
>> +       frame_ver = *buffer++;
>> +       frame_len = *buffer++;
>> +       checksum = *buffer++;
>>         frame_data = buffer;
>> +       if ((frame_len + HDMI_INFOFRAME_HEADER_SIZE) > len) {
>> +               dev_err(hdmi->dev, "Wrong frame len: %d\n", frame_len;
>> +               return;
>> +       }
>>
>>         dev_dbg(hdmi->dev,
>>                 "frame_type:0x%x,frame_ver:0x%x,frame_len:0x%x,checksum:0x%x\n",
>> @@ -982,7 +982,7 @@ static int mtk_hdmi_setup_avi_infoframe(struct mtk_hdmi *hdmi,
>>                                         struct drm_display_mode *mode)
>>  {
>>         struct hdmi_avi_infoframe frame;
>> -       u8 buffer[17];
>> +       u8 buffer[HDMI_INFOFRAME_HEADER_SIZE + HDMI_AVI_INFOFRAME_SIZE];
>>         ssize_t err;
>>
>>         err = drm_hdmi_avi_infoframe_from_display_mode(&frame,
>> @@ -1008,7 +1008,7 @@ static int mtk_hdmi_setup_spd_infoframe(struct mtk_hdmi *hdmi,
>>                                         const char *product)
>>  {
>>         struct hdmi_spd_infoframe frame;
>> -       u8 buffer[29];
>> +       u8 buffer[HDMI_INFOFRAME_HEADER_SIZE + HDMI_SPD_INFOFRAME_SIZE];
>>         ssize_t err;
>>
>>         err = hdmi_spd_infoframe_init(&frame, vendor, product);
>> @@ -1031,7 +1031,7 @@ static int mtk_hdmi_setup_spd_infoframe(struct mtk_hdmi *hdmi,
>>  static int mtk_hdmi_setup_audio_infoframe(struct mtk_hdmi *hdmi)
>>  {
>>         struct hdmi_audio_infoframe frame;
>> -       u8 buffer[14];
>> +       u8 buffer[HDMI_INFOFRAME_HEADER_SIZE + HDMI_AUDIO_INFOFRAME_SIZE];
>>         ssize_t err;
>>
>>         err = hdmi_audio_infoframe_init(&frame);
>> --
>> 2.26.2
>>
>>
>> _______________________________________________
>> Linux-mediatek mailing list
>> Linux-mediatek@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-mediatek
Chun-Kuang Hu April 30, 2020, 3:50 p.m. UTC | #3
Hi, Bernard:

Bernard <bernard@vivo.com> 於 2020年4月30日 週四 下午2:32寫道:
>
>
>
> 发件人:Chun-Kuang Hu <chunkuang.hu@kernel.org>
> 发送日期:2020-04-29 22:22:50
> 收件人:Bernard Zhao <bernard@vivo.com>
> 抄送人:Chun-Kuang Hu <chunkuang.hu@kernel.org>,Philipp Zabel <p.zabel@pengutronix.de>,David Airlie <airlied@linux.ie>,Daniel Vetter <daniel@ffwll.ch>,Matthias Brugger <matthias.bgg@gmail.com>,DRI Development <dri-devel@lists.freedesktop.org>,Linux ARM <linux-arm-kernel@lists.infradead.org>,"moderated list:ARM/Mediatek SoC support" <linux-mediatek@lists.infradead.org>,linux-kernel <linux-kernel@vger.kernel.org>,opensource.kernel@vivo.com
> 主题:Re: [PATCH] drm/mediatek: cleanup coding style in mediatek a bit>Hi, Bernard:
> >
> >Bernard Zhao <bernard@vivo.com> 於 2020年4月27日 週一 下午3:53寫道:
> >>
> >> This code change is to make code bit more readable.
> >> Optimise array size align to HDMI macro define.
> >> Add check if len is overange.
> >
> >One patch should just do one thing, but this do three things.
> >So break this into three patches.
> >
> >Regards,
> >Chun-Kuang.
>
> Hi
> This optimization is mainly to make the code a bit readable.
> These modifications are related, main in several related function calls in the same file.
> I was a bit confused that if it is really necessary to change to three separate patch submissions?
>
> Regards
> Bernard
>
> >>
> >> Signed-off-by: Bernard Zhao <bernard@vivo.com>
> >> ---
> >>  drivers/gpu/drm/mediatek/mtk_hdmi.c | 22 +++++++++++-----------
> >>  1 file changed, 11 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> >> index ff43a3d80410..40fb5154ed5d 100644
> >> --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> >> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> >> @@ -311,15 +311,15 @@ static void mtk_hdmi_hw_send_info_frame(struct mtk_hdmi *hdmi, u8 *buffer,
> >>         u8 checksum;
> >>         int ctrl_frame_en = 0;
> >>
> >> -       frame_type = *buffer;
> >> -       buffer += 1;
> >> -       frame_ver = *buffer;
> >> -       buffer += 1;
> >> -       frame_len = *buffer;
> >> -       buffer += 1;
> >> -       checksum = *buffer;
> >> -       buffer += 1;
> >> +       frame_type = *buffer++;
> >> +       frame_ver = *buffer++;
> >> +       frame_len = *buffer++;
> >> +       checksum = *buffer++;

This part looks like cleanup, so keep in this patch.

> >>         frame_data = buffer;
> >> +       if ((frame_len + HDMI_INFOFRAME_HEADER_SIZE) > len) {
> >> +               dev_err(hdmi->dev, "Wrong frame len: %d\n", frame_len;
> >> +               return;

This is error checking, not cleanup the coding style, so move this to
another patch.

> >> +       }
> >>
> >>         dev_dbg(hdmi->dev,
> >>                 "frame_type:0x%x,frame_ver:0x%x,frame_len:0x%x,checksum:0x%x\n",
> >> @@ -982,7 +982,7 @@ static int mtk_hdmi_setup_avi_infoframe(struct mtk_hdmi *hdmi,
> >>                                         struct drm_display_mode *mode)
> >>  {
> >>         struct hdmi_avi_infoframe frame;
> >> -       u8 buffer[17];
> >> +       u8 buffer[HDMI_INFOFRAME_HEADER_SIZE + HDMI_AVI_INFOFRAME_SIZE];

This is to symbolize the number, symbolization is more than cleanup.

Regards,
Chun-Kuang.

> >>         ssize_t err;
> >>
> >>         err = drm_hdmi_avi_infoframe_from_display_mode(&frame,
> >> @@ -1008,7 +1008,7 @@ static int mtk_hdmi_setup_spd_infoframe(struct mtk_hdmi *hdmi,
> >>                                         const char *product)
> >>  {
> >>         struct hdmi_spd_infoframe frame;
> >> -       u8 buffer[29];
> >> +       u8 buffer[HDMI_INFOFRAME_HEADER_SIZE + HDMI_SPD_INFOFRAME_SIZE];
> >>         ssize_t err;
> >>
> >>         err = hdmi_spd_infoframe_init(&frame, vendor, product);
> >> @@ -1031,7 +1031,7 @@ static int mtk_hdmi_setup_spd_infoframe(struct mtk_hdmi *hdmi,
> >>  static int mtk_hdmi_setup_audio_infoframe(struct mtk_hdmi *hdmi)
> >>  {
> >>         struct hdmi_audio_infoframe frame;
> >> -       u8 buffer[14];
> >> +       u8 buffer[HDMI_INFOFRAME_HEADER_SIZE + HDMI_AUDIO_INFOFRAME_SIZE];
> >>         ssize_t err;
> >>
> >>         err = hdmi_audio_infoframe_init(&frame);
> >> --
> >> 2.26.2
> >>
> >>
> >> _______________________________________________
> >> Linux-mediatek mailing list
> >> Linux-mediatek@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-mediatek
>
>
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
kernel test robot May 1, 2020, 3:57 a.m. UTC | #4
Hi Bernard,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on pza/reset/next]
[also build test ERROR on drm-intel/for-linux-next drm-tip/drm-tip linus/master v5.7-rc3 next-20200430]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Bernard-Zhao/drm-mediatek-cleanup-coding-style-in-mediatek-a-bit/20200428-055750
base:   https://git.pengutronix.de/git/pza/linux reset/next
config: arm64-allyesconfig (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day GCC_VERSION=9.3.0 make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/gpu/drm/mediatek/mtk_hdmi.c: In function 'mtk_hdmi_hw_send_info_frame':
>> drivers/gpu/drm/mediatek/mtk_hdmi.c:1807: error: unterminated argument list invoking macro "dev_err"
    1807 | MODULE_LICENSE("GPL v2");
         | 
>> drivers/gpu/drm/mediatek/mtk_hdmi.c:316:3: error: 'dev_err' undeclared (first use in this function); did you mean '_dev_err'?
     316 |   dev_err(hdmi->dev, "Wrong frame len: %d\n", frame_len;
         |   ^~~~~~~
         |   _dev_err
   drivers/gpu/drm/mediatek/mtk_hdmi.c:316:3: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/gpu/drm/mediatek/mtk_hdmi.c:316:10: error: expected ';' at end of input
     316 |   dev_err(hdmi->dev, "Wrong frame len: %d\n", frame_len;
         |          ^
         |          ;
   ......
    1807 | MODULE_LICENSE("GPL v2");
         |           
>> drivers/gpu/drm/mediatek/mtk_hdmi.c:316:3: error: expected declaration or statement at end of input
     316 |   dev_err(hdmi->dev, "Wrong frame len: %d\n", frame_len;
         |   ^~~~~~~
>> drivers/gpu/drm/mediatek/mtk_hdmi.c:316:3: error: expected declaration or statement at end of input
   drivers/gpu/drm/mediatek/mtk_hdmi.c:308:6: warning: unused variable 'ctrl_frame_en' [-Wunused-variable]
     308 |  int ctrl_frame_en = 0;
         |      ^~~~~~~~~~~~~
   drivers/gpu/drm/mediatek/mtk_hdmi.c:302:6: warning: unused variable 'i' [-Wunused-variable]
     302 |  int i;
         |      ^
   drivers/gpu/drm/mediatek/mtk_hdmi.c:301:6: warning: unused variable 'ctrl_reg' [-Wunused-variable]
     301 |  u32 ctrl_reg = GRL_CTRL;
         |      ^~~~~~~~
   At top level:
   drivers/gpu/drm/mediatek/mtk_hdmi.c:298:13: warning: 'mtk_hdmi_hw_send_info_frame' defined but not used [-Wunused-function]
     298 | static void mtk_hdmi_hw_send_info_frame(struct mtk_hdmi *hdmi, u8 *buffer,
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/mediatek/mtk_hdmi.c:293:13: warning: 'mtk_hdmi_hw_enable_dvi_mode' defined but not used [-Wunused-function]
     293 | static void mtk_hdmi_hw_enable_dvi_mode(struct mtk_hdmi *hdmi, bool enable)
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/mediatek/mtk_hdmi.c:288:13: warning: 'mtk_hdmi_hw_write_int_mask' defined but not used [-Wunused-function]
     288 | static void mtk_hdmi_hw_write_int_mask(struct mtk_hdmi *hdmi, u32 int_mask)
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/mediatek/mtk_hdmi.c:282:13: warning: 'mtk_hdmi_hw_enable_notice' defined but not used [-Wunused-function]
     282 | static void mtk_hdmi_hw_enable_notice(struct mtk_hdmi *hdmi, bool enable_notice)
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/mediatek/mtk_hdmi.c:271:13: warning: 'mtk_hdmi_hw_reset' defined but not used [-Wunused-function]
     271 | static void mtk_hdmi_hw_reset(struct mtk_hdmi *hdmi)
         |             ^~~~~~~~~~~~~~~~~
   drivers/gpu/drm/mediatek/mtk_hdmi.c:266:13: warning: 'mtk_hdmi_hw_aud_unmute' defined but not used [-Wunused-function]
     266 | static void mtk_hdmi_hw_aud_unmute(struct mtk_hdmi *hdmi)
         |             ^~~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/mediatek/mtk_hdmi.c:261:13: warning: 'mtk_hdmi_hw_aud_mute' defined but not used [-Wunused-function]
     261 | static void mtk_hdmi_hw_aud_mute(struct mtk_hdmi *hdmi)
         |             ^~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/mediatek/mtk_hdmi.c:255:13: warning: 'mtk_hdmi_hw_1p4_version_enable' defined but not used [-Wunused-function]
     255 | static void mtk_hdmi_hw_1p4_version_enable(struct mtk_hdmi *hdmi, bool enable)
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/mediatek/mtk_hdmi.c:230:13: warning: 'mtk_hdmi_hw_make_reg_writable' defined but not used [-Wunused-function]
     230 | static void mtk_hdmi_hw_make_reg_writable(struct mtk_hdmi *hdmi, bool enable)
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/mediatek/mtk_hdmi.c:224:13: warning: 'mtk_hdmi_hw_vid_black' defined but not used [-Wunused-function]
     224 | static void mtk_hdmi_hw_vid_black(struct mtk_hdmi *hdmi, bool black)
         |             ^~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/mediatek/mtk_hdmi.c:184:12: warning: 'mtk_hdmi_read' defined but not used [-Wunused-function]
     184 | static u32 mtk_hdmi_read(struct mtk_hdmi *hdmi, u32 offset)
         |            ^~~~~~~~~~~~~

vim +/dev_err +1807 drivers/gpu/drm/mediatek/mtk_hdmi.c

8f83f26891e1257 Jie Qiu 2016-01-04  1804  
8f83f26891e1257 Jie Qiu 2016-01-04  1805  MODULE_AUTHOR("Jie Qiu <jie.qiu@mediatek.com>");
8f83f26891e1257 Jie Qiu 2016-01-04  1806  MODULE_DESCRIPTION("MediaTek HDMI Driver");
8f83f26891e1257 Jie Qiu 2016-01-04 @1807  MODULE_LICENSE("GPL v2");

:::::: The code at line 1807 was first introduced by commit
:::::: 8f83f26891e12570780dcfc8ae376b655915ff6d drm/mediatek: Add HDMI support

:::::: TO: Jie Qiu <jie.qiu@mediatek.com>
:::::: CC: Philipp Zabel <p.zabel@pengutronix.de>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Bernard Zhao May 6, 2020, 12:14 p.m. UTC | #5
发件人:Chun-Kuang Hu <chunkuang.hu@kernel.org>
发送日期:2020-04-30 23:50:38
收件人:Bernard <bernard@vivo.com>
抄送人:Chun-Kuang Hu <chunkuang.hu@kernel.org>,Philipp Zabel <p.zabel@pengutronix.de>,opensource.kernel@vivo.com,David Airlie <airlied@linux.ie>,linux-kernel <linux-kernel@vger.kernel.org>,DRI Development <dri-devel@lists.freedesktop.org>,"moderated list:ARM/Mediatek SoC support" <linux-mediatek@lists.infradead.org>,Daniel Vetter <daniel@ffwll.ch>,Matthias Brugger <matthias.bgg@gmail.com>,Linux ARM <linux-arm-kernel@lists.infradead.org>
主题:Re: Re: [PATCH] drm/mediatek: cleanup coding style in mediatek a bit>Hi, Bernard:
>
>Bernard <bernard@vivo.com> 於 2020年4月30日 週四 下午2:32寫道:
>>
>>
>>
>> 发件人:Chun-Kuang Hu <chunkuang.hu@kernel.org>
>> 发送日期:2020-04-29 22:22:50
>> 收件人:Bernard Zhao <bernard@vivo.com>
>> 抄送人:Chun-Kuang Hu <chunkuang.hu@kernel.org>,Philipp Zabel <p.zabel@pengutronix.de>,David Airlie <airlied@linux.ie>,Daniel Vetter <daniel@ffwll.ch>,Matthias Brugger <matthias.bgg@gmail.com>,DRI Development <dri-devel@lists.freedesktop.org>,Linux ARM <linux-arm-kernel@lists.infradead.org>,"moderated list:ARM/Mediatek SoC support" <linux-mediatek@lists.infradead.org>,linux-kernel <linux-kernel@vger.kernel.org>,opensource.kernel@vivo.com
>> 主题:Re: [PATCH] drm/mediatek: cleanup coding style in mediatek a bit>Hi, Bernard:
>> >
>> >Bernard Zhao <bernard@vivo.com> 於 2020年4月27日 週一 下午3:53寫道:
>> >>
>> >> This code change is to make code bit more readable.
>> >> Optimise array size align to HDMI macro define.
>> >> Add check if len is overange.
>> >
>> >One patch should just do one thing, but this do three things.
>> >So break this into three patches.
>> >
>> >Regards,
>> >Chun-Kuang.
>>
>> Hi
>> This optimization is mainly to make the code a bit readable.
>> These modifications are related, main in several related function calls in the same file.
>> I was a bit confused that if it is really necessary to change to three separate patch submissions?
>>
>> Regards
>> Bernard
>>
>> >>
>> >> Signed-off-by: Bernard Zhao <bernard@vivo.com>
>> >> ---
>> >>  drivers/gpu/drm/mediatek/mtk_hdmi.c | 22 +++++++++++-----------
>> >>  1 file changed, 11 insertions(+), 11 deletions(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
>> >> index ff43a3d80410..40fb5154ed5d 100644
>> >> --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
>> >> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
>> >> @@ -311,15 +311,15 @@ static void mtk_hdmi_hw_send_info_frame(struct mtk_hdmi *hdmi, u8 *buffer,
>> >>         u8 checksum;
>> >>         int ctrl_frame_en = 0;
>> >>
>> >> -       frame_type = *buffer;
>> >> -       buffer += 1;
>> >> -       frame_ver = *buffer;
>> >> -       buffer += 1;
>> >> -       frame_len = *buffer;
>> >> -       buffer += 1;
>> >> -       checksum = *buffer;
>> >> -       buffer += 1;
>> >> +       frame_type = *buffer++;
>> >> +       frame_ver = *buffer++;
>> >> +       frame_len = *buffer++;
>> >> +       checksum = *buffer++;
>
>This part looks like cleanup, so keep in this patch.
>
>> >>         frame_data = buffer;
>> >> +       if ((frame_len + HDMI_INFOFRAME_HEADER_SIZE) > len) {
>> >> +               dev_err(hdmi->dev, "Wrong frame len: %d\n", frame_len;
>> >> +               return;
>
>This is error checking, not cleanup the coding style, so move this to
>another patch.
>
>> >> +       }
>> >>
>> >>         dev_dbg(hdmi->dev,
>> >>                 "frame_type:0x%x,frame_ver:0x%x,frame_len:0x%x,checksum:0x%x\n",
>> >> @@ -982,7 +982,7 @@ static int mtk_hdmi_setup_avi_infoframe(struct mtk_hdmi *hdmi,
>> >>                                         struct drm_display_mode *mode)
>> >>  {
>> >>         struct hdmi_avi_infoframe frame;
>> >> -       u8 buffer[17];
>> >> +       u8 buffer[HDMI_INFOFRAME_HEADER_SIZE + HDMI_AVI_INFOFRAME_SIZE];
>
>This is to symbolize the number, symbolization is more than cleanup.
>
>Regards,
>Chun-Kuang.
>

Hi
Sure, I get the picture, i will resubmit, thank you!

Regards,
Bernard

>> >>         ssize_t err;
>> >>
>> >>         err = drm_hdmi_avi_infoframe_from_display_mode(&frame,
>> >> @@ -1008,7 +1008,7 @@ static int mtk_hdmi_setup_spd_infoframe(struct mtk_hdmi *hdmi,
>> >>                                         const char *product)
>> >>  {
>> >>         struct hdmi_spd_infoframe frame;
>> >> -       u8 buffer[29];
>> >> +       u8 buffer[HDMI_INFOFRAME_HEADER_SIZE + HDMI_SPD_INFOFRAME_SIZE];
>> >>         ssize_t err;
>> >>
>> >>         err = hdmi_spd_infoframe_init(&frame, vendor, product);
>> >> @@ -1031,7 +1031,7 @@ static int mtk_hdmi_setup_spd_infoframe(struct mtk_hdmi *hdmi,
>> >>  static int mtk_hdmi_setup_audio_infoframe(struct mtk_hdmi *hdmi)
>> >>  {
>> >>         struct hdmi_audio_infoframe frame;
>> >> -       u8 buffer[14];
>> >> +       u8 buffer[HDMI_INFOFRAME_HEADER_SIZE + HDMI_AUDIO_INFOFRAME_SIZE];
>> >>         ssize_t err;
>> >>
>> >>         err = hdmi_audio_infoframe_init(&frame);
>> >> --
>> >> 2.26.2
>> >>
>> >>
>> >> _______________________________________________
>> >> Linux-mediatek mailing list
>> >> Linux-mediatek@lists.infradead.org
>> >> http://lists.infradead.org/mailman/listinfo/linux-mediatek
>>
>>
>> _______________________________________________
>> Linux-mediatek mailing list
>> Linux-mediatek@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-mediatek
diff mbox series

Patch

diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
index ff43a3d80410..40fb5154ed5d 100644
--- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
+++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
@@ -311,15 +311,15 @@  static void mtk_hdmi_hw_send_info_frame(struct mtk_hdmi *hdmi, u8 *buffer,
 	u8 checksum;
 	int ctrl_frame_en = 0;
 
-	frame_type = *buffer;
-	buffer += 1;
-	frame_ver = *buffer;
-	buffer += 1;
-	frame_len = *buffer;
-	buffer += 1;
-	checksum = *buffer;
-	buffer += 1;
+	frame_type = *buffer++;
+	frame_ver = *buffer++;
+	frame_len = *buffer++;
+	checksum = *buffer++;
 	frame_data = buffer;
+	if ((frame_len + HDMI_INFOFRAME_HEADER_SIZE) > len) {
+		dev_err(hdmi->dev, "Wrong frame len: %d\n", frame_len;
+		return;
+	}
 
 	dev_dbg(hdmi->dev,
 		"frame_type:0x%x,frame_ver:0x%x,frame_len:0x%x,checksum:0x%x\n",
@@ -982,7 +982,7 @@  static int mtk_hdmi_setup_avi_infoframe(struct mtk_hdmi *hdmi,
 					struct drm_display_mode *mode)
 {
 	struct hdmi_avi_infoframe frame;
-	u8 buffer[17];
+	u8 buffer[HDMI_INFOFRAME_HEADER_SIZE + HDMI_AVI_INFOFRAME_SIZE];
 	ssize_t err;
 
 	err = drm_hdmi_avi_infoframe_from_display_mode(&frame,
@@ -1008,7 +1008,7 @@  static int mtk_hdmi_setup_spd_infoframe(struct mtk_hdmi *hdmi,
 					const char *product)
 {
 	struct hdmi_spd_infoframe frame;
-	u8 buffer[29];
+	u8 buffer[HDMI_INFOFRAME_HEADER_SIZE + HDMI_SPD_INFOFRAME_SIZE];
 	ssize_t err;
 
 	err = hdmi_spd_infoframe_init(&frame, vendor, product);
@@ -1031,7 +1031,7 @@  static int mtk_hdmi_setup_spd_infoframe(struct mtk_hdmi *hdmi,
 static int mtk_hdmi_setup_audio_infoframe(struct mtk_hdmi *hdmi)
 {
 	struct hdmi_audio_infoframe frame;
-	u8 buffer[14];
+	u8 buffer[HDMI_INFOFRAME_HEADER_SIZE + HDMI_AUDIO_INFOFRAME_SIZE];
 	ssize_t err;
 
 	err = hdmi_audio_infoframe_init(&frame);