Re:Re: Re: [PATCH] drm/mediatek: cleanup coding style in mediatek a bit

2020-05-07 Thread Bernard 


发件人：Chun-Kuang Hu 
发送日期：2020-04-30 23:50:38
收件人：Bernard 
抄送人：Chun-Kuang Hu ,Philipp Zabel 
,opensource.ker...@vivo.com,David Airlie 
,linux-kernel ,DRI Development 
,"moderated list:ARM/Mediatek SoC support" 
,Daniel Vetter ,Matthias 
Brugger ,Linux ARM 

主题：Re: Re: [PATCH] drm/mediatek: cleanup coding style in mediatek a bit>Hi, 
Bernard:
>
>Bernard  於 2020年4月30日 週四 下午2:32寫道：
>>
>>
>>
>> 发件人：Chun-Kuang Hu 
>> 发送日期：2020-04-29 22:22:50
>> 收件人：Bernard Zhao 
>> 抄送人：Chun-Kuang Hu ,Philipp Zabel 
>> ,David Airlie ,Daniel Vetter 
>> ,Matthias Brugger ,DRI Development 
>> ,Linux ARM 
>> ,"moderated list:ARM/Mediatek SoC 
>> support" ,linux-kernel 
>> ,opensource.ker...@vivo.com
>> 主题：Re: [PATCH] drm/mediatek: cleanup coding style in mediatek a bit>Hi, 
>> Bernard:
>> >
>> >Bernard Zhao  於 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 
>> >> ---
>> >>  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(,
>> >> @@ -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(, 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();
>> >> --
>> >> 2.26.2
>> >>
>> >>
>> >> ___
>> >> Linux-mediatek mailing list
>> >> linux-media...@lists.infradead.org
>> >> http://lists.infradead.org/mailman/listinfo/linux-mediatek
>>
>>
>> ___
>> Linux-mediatek mailing list
>> linux-media...@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-mediatek


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/mediatek: cleanup coding style in mediatek a bit

2020-04-30 Thread kbuild test robot 
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 

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

Re: Re: [PATCH] drm/mediatek: cleanup coding style in mediatek a bit

2020-04-30 Thread Chun-Kuang Hu 
Hi, Bernard:

Bernard  於 2020年4月30日 週四 下午2:32寫道：
>
>
>
> 发件人：Chun-Kuang Hu 
> 发送日期：2020-04-29 22:22:50
> 收件人：Bernard Zhao 
> 抄送人：Chun-Kuang Hu ,Philipp Zabel 
> ,David Airlie ,Daniel Vetter 
> ,Matthias Brugger ,DRI Development 
> ,Linux ARM 
> ,"moderated list:ARM/Mediatek SoC 
> support" ,linux-kernel 
> ,opensource.ker...@vivo.com
> 主题：Re: [PATCH] drm/mediatek: cleanup coding style in mediatek a bit>Hi, 
> Bernard:
> >
> >Bernard Zhao  於 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 
> >> ---
> >>  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(,
> >> @@ -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(, 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();
> >> --
> >> 2.26.2
> >>
> >>
> >> ___
> >> Linux-mediatek mailing list
> >> linux-media...@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-mediatek
>
>
> ___
> Linux-mediatek mailing list
> linux-media...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/mediatek: cleanup coding style in mediatek a bit

2020-04-29 Thread Chun-Kuang Hu 
Hi, Bernard:

Bernard Zhao  於 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 
> ---
>  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(,
> @@ -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(, 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();
> --
> 2.26.2
>
>
> ___
> Linux-mediatek mailing list
> linux-media...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/mediatek: cleanup coding style in mediatek a bit

2020-04-28 Thread Markus Elfring 
> This code change is to make code bit more readable.
> Optimise array size align to HDMI macro define.
> Add check if len is overange.

I find it safer to handle also such source code adjustments
by a small patch series together with improved commit messages.

Regards,
Markus
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel