lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 03 Sep 2015 13:33:27 +0800 From: Yakir Yang <ykk@...k-chips.com> To: Krzysztof Kozlowski <k.kozlowski@...sung.com>, Heiko Stuebner <heiko@...ech.de>, Thierry Reding <treding@...dia.com>, Jingoo Han <jingoohan1@...il.com>, Inki Dae <inki.dae@...sung.com>, joe@...ches.com, Kukjin Kim <kgene@...nel.org>, Mark Yao <mark.yao@...k-chips.com> CC: Russell King <rmk+kernel@....linux.org.uk>, djkurtz@...omium.com, dianders@...omium.com, seanpaul@...omium.com, ajaynumb@...il.com, Andrzej Hajda <a.hajda@...sung.com>, Kyungmin Park <kyungmin.park@...sung.com>, David Airlie <airlied@...ux.ie>, Gustavo Padovan <gustavo.padovan@...labora.co.uk>, Andy Yan <andy.yan@...k-chips.com>, Kumar Gala <galak@...eaurora.org>, Ian Campbell <ijc+devicetree@...lion.org.uk>, Rob Herring <robh+dt@...nel.org>, Pawel Moll <pawel.moll@....com>, Kishon Vijay Abraham I <kishon@...com>, architt@...eaurora.org, robherring2@...il.com, dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, linux-samsung-soc@...r.kernel.org, linux-rockchip@...ts.infradead.org, linux-arm-kernel@...ts.infradead.org Subject: Re: [PATCH v4 01/16] drm: exynos/dp: fix code style Hi Krzysztof, 在 09/03/2015 01:08 PM, Krzysztof Kozlowski 写道: > On 03.09.2015 14:04, Yakir Yang wrote: >> Hi Krzysztof, >> >> 在 09/03/2015 08:21 AM, Krzysztof Kozlowski 写道: >>> On 01.09.2015 14:46, Yakir Yang wrote: >>>> After run "checkpatch.pl -f --subjective" command, I see there >>>> are lots of alignment problem in exynos_dp driver, so let just >>>> fix them. >>> Hi, >>> >>> Warnings from checkpatch are not a reason for a commit. Reason for a >>> commit could be for example an unreadable code, violation of >>> coding-style leading to decrease in code maintainability or just >>> improving the code readability so it will be easier to review and >>> maintain it. >>> >>> You do not make commits because some tool tells you that. We do not >>> listen to machines :) ... If that would be the case, the commit could be >>> made automatically, without human interaction. Such automated commit >>> could be even easily tested by the machine by comparing object files. >>> >>> Especially that you enabled "subjective" rule. This is not a valid >>> motivation for a commit. >>> >>> Please rephrase this to sensible reason and convince that change is >>> worth the effort. >> Oh, nice, thanks for your remind. I would rephrase the commit. >> >>>> - Take Romain suggest, rebase on linux-next branch >>> That comment seems unrelated to the commit. Please remove it. >> Done, >> >>>> Signed-off-by: Yakir Yang <ykk@...k-chips.com> >>>> --- >>>> Changes in v4: None >>>> Changes in v3: None >>>> Changes in v2: >>>> - Take Joe Preches advise, improved commit message more readable, and >>>> avoid using some uncommon style like bellow: >>>> - retval = exynos_dp_read_bytes_from_i2c(... >>>> ...) >>>> + retval = >>>> + exynos_dp_read_bytes_from_i2c(......); >>>> >>>> drivers/gpu/drm/exynos/exynos_dp_core.c | 226 >>>> ++++++++++++++++---------------- >>>> drivers/gpu/drm/exynos/exynos_dp_core.h | 54 ++++---- >>>> drivers/gpu/drm/exynos/exynos_dp_reg.c | 106 +++++++-------- >>>> 3 files changed, 188 insertions(+), 198 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c >>>> b/drivers/gpu/drm/exynos/exynos_dp_core.c >>>> index d66ade0..266f7f7 100644 >>>> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c >>>> +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c >>>> @@ -115,8 +115,8 @@ static int exynos_dp_read_edid(struct >>>> exynos_dp_device *dp) >>>> /* Read Extension Flag, Number of 128-byte EDID extension >>>> blocks */ >>>> retval = exynos_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR, >>>> - EDID_EXTENSION_FLAG, >>>> - &extend_block); >>>> + EDID_EXTENSION_FLAG, >>>> + &extend_block); >>>> if (retval) >>>> return retval; >>>> @@ -124,10 +124,11 @@ static int exynos_dp_read_edid(struct >>>> exynos_dp_device *dp) >>>> dev_dbg(dp->dev, "EDID data includes a single extension!\n"); >>>> /* Read EDID data */ >>>> - retval = exynos_dp_read_bytes_from_i2c(dp, >>>> I2C_EDID_DEVICE_ADDR, >>>> - EDID_HEADER_PATTERN, >>>> - EDID_BLOCK_LENGTH, >>>> - &edid[EDID_HEADER_PATTERN]); >>>> + retval = exynos_dp_read_bytes_from_i2c( >>>> + dp, I2C_EDID_DEVICE_ADDR, >>>> + EDID_HEADER_PATTERN, >>>> + EDID_BLOCK_LENGTH, >>>> + &edid[EDID_HEADER_PATTERN]); >>>> if (retval != 0) { >>>> dev_err(dp->dev, "EDID Read failed!\n"); >>>> return -EIO; >>>> @@ -139,11 +140,11 @@ static int exynos_dp_read_edid(struct >>>> exynos_dp_device *dp) >>>> } >>>> /* Read additional EDID data */ >>>> - retval = exynos_dp_read_bytes_from_i2c(dp, >>>> - I2C_EDID_DEVICE_ADDR, >>>> - EDID_BLOCK_LENGTH, >>>> - EDID_BLOCK_LENGTH, >>>> - &edid[EDID_BLOCK_LENGTH]); >>>> + retval = exynos_dp_read_bytes_from_i2c( >>>> + dp, I2C_EDID_DEVICE_ADDR, >>>> + EDID_BLOCK_LENGTH, >>>> + EDID_BLOCK_LENGTH, >>>> + &edid[EDID_BLOCK_LENGTH]); >>>> if (retval != 0) { >>>> dev_err(dp->dev, "EDID Read failed!\n"); >>>> return -EIO; >>>> @@ -155,24 +156,22 @@ static int exynos_dp_read_edid(struct >>>> exynos_dp_device *dp) >>>> } >>>> exynos_dp_read_byte_from_dpcd(dp, DP_TEST_REQUEST, >>>> - &test_vector); >>>> + &test_vector); >>>> if (test_vector & DP_TEST_LINK_EDID_READ) { >>>> - exynos_dp_write_byte_to_dpcd(dp, >>>> - DP_TEST_EDID_CHECKSUM, >>>> + exynos_dp_write_byte_to_dpcd( >>>> + dp, DP_TEST_EDID_CHECKSUM, >>>> edid[EDID_BLOCK_LENGTH + EDID_CHECKSUM]); >>>> - exynos_dp_write_byte_to_dpcd(dp, >>>> - DP_TEST_RESPONSE, >>>> + exynos_dp_write_byte_to_dpcd( >>>> + dp, DP_TEST_RESPONSE, >>>> DP_TEST_EDID_CHECKSUM_WRITE); >>> To me, missing argument after opening parenthesis, looks worse. I would >>> prefer: >>> >>> exynos_dp_write_byte_to_dpcd(dp, >>> >>> Why you moved the 'dp' argument to new line? >> Hmm... Just like style tool indicate, no more warning after >> that change. >> >> For now, I would like to follow the original style, just improved >> some obvious style problem. :-) > What was the checkpatch warning that said 'dp' has to move to new line? > I tried this and I don't see it. checkpatch haven't remind me that put dp to new line would fix this warning, this just come from my experiments. And I works, no more warnings from checkpatch, so I toke this style. - Yakir > Best regards, > Krzysztof > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists