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 <[email protected]>
---
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
> 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
Hi, Bernard:
Bernard Zhao <[email protected]> 於 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 <[email protected]>
> ---
> 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
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
发件人：Chun-Kuang Hu <[email protected]>
发送日期：2020-04-29 22:22:50
收件人：Bernard Zhao <[email protected]>
抄送人：Chun-Kuang Hu <[email protected]>,Philipp Zabel <[email protected]>,David Airlie <[email protected]>,Daniel Vetter <[email protected]>,Matthias Brugger <[email protected]>,DRI Development <[email protected]>,Linux ARM <[email protected]>,"moderated list:ARM/Mediatek SoC support" <[email protected]>,linux-kernel <[email protected]>,[email protected]
主题：Re: [PATCH] drm/mediatek: cleanup coding style in mediatek a bit>Hi, Bernard:
>
>Bernard Zhao <[email protected]> 於 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 <[email protected]>
>> ---
>> 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
>> [email protected]
>> http://lists.infradead.org/mailman/listinfo/linux-mediatek
Hi, Bernard:
Bernard <[email protected]> 於 2020年4月30日 週四 下午2:32寫道：
>
>
>
> 发件人：Chun-Kuang Hu <[email protected]>
> 发送日期：2020-04-29 22:22:50
> 收件人：Bernard Zhao <[email protected]>
> 抄送人：Chun-Kuang Hu <[email protected]>,Philipp Zabel <[email protected]>,David Airlie <[email protected]>,Daniel Vetter <[email protected]>,Matthias Brugger <[email protected]>,DRI Development <[email protected]>,Linux ARM <[email protected]>,"moderated list:ARM/Mediatek SoC support" <[email protected]>,linux-kernel <[email protected]>,[email protected]
> 主题：Re: [PATCH] drm/mediatek: cleanup coding style in mediatek a bit>Hi, Bernard:
> >
> >Bernard Zhao <[email protected]> 於 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 <[email protected]>
> >> ---
> >> 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
> >> [email protected]
> >> http://lists.infradead.org/mailman/listinfo/linux-mediatek
>
>
> _______________________________________________
> Linux-mediatek mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
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 <[email protected]>
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 <[email protected]>");
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 <[email protected]>
:::::: CC: Philipp Zabel <[email protected]>
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
发件人：Chun-Kuang Hu <[email protected]>
发送日期：2020-04-30 23:50:38
收件人：Bernard <[email protected]>
抄送人：Chun-Kuang Hu <[email protected]>,Philipp Zabel <[email protected]>,[email protected],David Airlie <[email protected]>,linux-kernel <[email protected]>,DRI Development <[email protected]>,"moderated list:ARM/Mediatek SoC support" <[email protected]>,Daniel Vetter <[email protected]>,Matthias Brugger <[email protected]>,Linux ARM <[email protected]>
主题：Re: Re: [PATCH] drm/mediatek: cleanup coding style in mediatek a bit>Hi, Bernard:
>
>Bernard <[email protected]> 於 2020年4月30日 週四 下午2:32寫道：
>>
>>
>>
>> 发件人：Chun-Kuang Hu <[email protected]>
>> 发送日期：2020-04-29 22:22:50
>> 收件人：Bernard Zhao <[email protected]>
>> 抄送人：Chun-Kuang Hu <[email protected]>,Philipp Zabel <[email protected]>,David Airlie <[email protected]>,Daniel Vetter <[email protected]>,Matthias Brugger <[email protected]>,DRI Development <[email protected]>,Linux ARM <[email protected]>,"moderated list:ARM/Mediatek SoC support" <[email protected]>,linux-kernel <[email protected]>,[email protected]
>> 主题：Re: [PATCH] drm/mediatek: cleanup coding style in mediatek a bit>Hi, Bernard:
>> >
>> >Bernard Zhao <[email protected]> 於 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 <[email protected]>
>> >> ---
>> >> 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
>> >> [email protected]
>> >> http://lists.infradead.org/mailman/listinfo/linux-mediatek
>>
>>
>> _______________________________________________
>> Linux-mediatek mailing list
>> [email protected]
>> http://lists.infradead.org/mailman/listinfo/linux-mediatek