From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752613AbdLFOxY (ORCPT ); Wed, 6 Dec 2017 09:53:24 -0500 Received: from mailout2.w1.samsung.com ([210.118.77.12]:33225 "EHLO mailout2.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752484AbdLFOxQ (ORCPT ); Wed, 6 Dec 2017 09:53:16 -0500 DKIM-Filter: OpenDKIM Filter v2.11.0 mailout2.w1.samsung.com 20171206145313euoutp026dd0cbbbd15381d811409f8fa8609d2f~9vDbiRIQ42453924539euoutp02- X-AuditID: cbfec7f5-f79d06d0000031c7-29-5a28045863d4 From: =?utf-8?Q?=C5=81ukasz_Stelmach?= To: Krzysztof Kozlowski Cc: robh+dt@kernel.org, Stephan Mueller , Herbert Xu , "David S. Miller" , Kukjin Kim , linux-crypto@vger.kernel.org, linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org, Marek Szyprowski , =?utf-8?Q?Bart=C5=82omiej_=C5=BBo?= =?utf-8?Q?=C5=82nierkiewicz?= Subject: Re: [PATCH 1/3] crypto: exynos - Support Exynos5250+ SoCs In-reply-to: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) Date: Wed, 06 Dec 2017 15:53:02 +0100 Message-id: <87374naoxd.fsf%l.stelmach@samsung.com> MIME-version: 1.0 Content-type: multipart/signed; boundary="=-=-="; micalg="pgp-sha256"; protocol="application/pgp-signature" X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrGKsWRmVeSWpSXmKPExsWy7djP87oRLBpRBv17DSw2zljPajHnfAuL RfcrGYv+x6+ZLc6f38Bucf/eTyaLy7vmsFnMOL+PyWLtkbvsFq17j7BbTD29lsWB2+PTlStM HltW3mTy2HZA1WPTqk42j74tqxg9Pm+SC2CL4rJJSc3JLEst0rdL4Mq4NfsYU8E9vYr3Rxex NTBOUe1i5OSQEDCRWPThMhuELSZx4d56IJuLQ0hgKaPEzw3XWCGcz4wSLy7MYoHpuD3hERNE YhmjxNHdfVAtXxgl9p5/zghSxSZgL9F/ZB9Yh4iApsT1v9/BRjELLGaW+HZ0MdhCYQEnib7O i+wgNqdAsMSF/klgtqiApcS9vrtgNSwCqhIn2maADeIVMJaYP7sByhaU+DH5HpjNLJAr8anp PzvIAgmBbnaJSTNboD5ykXjwvg3KFpZ4dXwLO4QtI3F5cjcLREM/o8Th+d+hElMYJRYvdICw rSX+rJrIBrGBT2LStunMXYwcQHFeiY42IYgSD4kjzw5BtTpK9HVMZYQExRJGidXfzjBPYJSd heTYWUiOnQU0ihkYMut36UOEtSWWLXzNDGHbSqxb955lASPrKkaR1NLi3PTUYlO94sTc4tK8 dL3k/NxNjMAUdPrf8a87GJceszrEKMDBqMTDe+GlepQQa2JZcWXuIUYVoDGPNqy+wCjFkpef l6okwnv5MlCaNyWxsiq1KD++qDQntfgQozQHi5I4r21UW6SQQHpiSWp2ampBahFMlomDU6qB UXdi2l1LgVcf0nYtmPI7UqOwc1X/t+OCB6y1ctbY3+fxPrOkOXZfrkzk/YUm0yLr9vA8vLiK Y8vf2Xnvnkqlhv/zyS29vXvijre3yzyf5058wZDd2u3jxcxaUTqhaFPtlT3bzlWduPdq8/VT no9Mn2krvjv0rERgqp5gb3GeSQOX5rZXdWcMZZRYijMSDbWYi4oTAR+yMnxJAwAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFupikeLIzCtJLcpLzFFi42I5/e/4Vd0IFo0og+7DAhYbZ6xntZhzvoXF ovuVjEX/49fMFufPb2C3uH/vJ5PF5V1z2CxmnN/HZLH2yF12i9a9R9gtpp5ey+LA7fHpyhUm jy0rbzJ5bDug6rFpVSebR9+WVYwenzfJBbBFcdmkpOZklqUW6dslcGXcmn2MqeCeXsX7o4vY GhinqHYxcnJICJhI3J7wiAnCFpO4cG89WxcjF4eQwBJGiY+rHjFCON8YJbbuf8AKUsUmYC/R f2QfC4gtIqApcf3vd1aQImaBpcwSHzfcZgdJCAs4SfR1XgSzOQWCJTau3Qy2QkggQGLm80Ng cVEBS4l7fXfZQGwWAVWJE20zwIbyChhLzJ/dAGULSvyYfA/MZhbIlrhw8Q3LBEb+WUhSs5Ck ZjFyANmaEut36UOEtSWWLXzNDGHbSqxb955lASPrKkaR1NLi3PTcYiO94sTc4tK8dL3k/NxN jMBY2Xbs55YdjF3vgg8xCnAwKvHwXnipHiXEmlhWXJl7iFEFaMyjDasvMEqx5OXnpSqJ8F6+ DJTmTUmsrEotyo8vKs1JLT7EKM3BoiTO27tndaSQQHpiSWp2ampBahFMlomDU6qBseiVxAPd xs+L++vmvj+hJy0Wc3DhtV0nvQ5OXBZ8oOuObeuNR93bXzzq+s377uKUhlLGb/MM/miekjiU 7brrxJf+kG7hxIXz02SuPRadty3qk8Cj33civXfp/l5m/6t2t8RGhZ8PCpOeV8p71Jdb/7j2 Lmdh6K3WoPnvpR7wHzEsK+Y+Z7vZO0KJpTgj0VCLuag4EQCRNiEqnQIAAA== X-CMS-MailID: 20171206145312eucas1p226d52f60f15e45456aefd6270cc88e07 X-Msg-Generator: CA CMS-TYPE: 201P X-CMS-RootMailID: 20171206145312eucas1p226d52f60f15e45456aefd6270cc88e07 X-RootMTR: 20171206145312eucas1p226d52f60f15e45456aefd6270cc88e07 References: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-=-= Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable It was <2017-12-06 =C5=9Bro 15:05>, when Krzysztof Kozlowski wrote: > On Wed, Dec 6, 2017 at 2:42 PM, =C5=81ukasz Stelmach wrote: >> It was <2017-12-05 wto 14:34>, when Krzysztof Kozlowski wrote: >>> On Tue, Dec 5, 2017 at 1:35 PM, =C5=81ukasz Stelmach wrote: >>>> Add support for PRNG in Exynos5250+ SoCs. >>>> >>>> Signed-off-by: =C5=81ukasz Stelmach >>>> --- >>>> .../bindings/crypto/samsung,exynos-rng4.txt | 4 ++- >>>> drivers/crypto/exynos-rng.c | 36 +++++++++++++= +++++++-- >>>> 2 files changed, 36 insertions(+), 4 deletions(-) >>>> >>>> diff --git >>>> a/Documentation/devicetree/bindings/crypto/samsung,exynos-rng4.txt >>>> b/Documentation/devicetree/bindings/crypto/samsung,exynos-rng4.txt >>>> index 4ca8dd4d7e66..a13fbdb4bd88 100644 >>>> --- a/Documentation/devicetree/bindings/crypto/samsung,exynos-rng4.txt >>>> +++ b/Documentation/devicetree/bindings/crypto/samsung,exynos-rng4.txt >>>> @@ -2,7 +2,9 @@ Exynos Pseudo Random Number Generator >>>> >>>> Required properties: >>>> >>>> -- compatible : Should be "samsung,exynos4-rng". >>>> +- compatible : One of: >>>> + - "samsung,exynos4-rng" for Exynos4210 and Exynos4412 >>>> + - "samsung,exynos5250-prng" for Exynos5250+ >>>> - reg : Specifies base physical address and size of the regis= ters map. >>>> - clocks : Phandle to clock-controller plus clock-specifier pair. >>>> - clock-names : "secss" as a clock name. >>>> diff --git a/drivers/crypto/exynos-rng.c b/drivers/crypto/exynos-rng.c >>>> index 451620b475a0..894ef93ef5ec 100644 >>>> --- a/drivers/crypto/exynos-rng.c >>>> +++ b/drivers/crypto/exynos-rng.c >>>> @@ -22,12 +22,17 @@ >>>> #include >>>> #include >>>> #include >>>> +#include >>>> #include >>>> >>>> #include >>>> >>>> #define EXYNOS_RNG_CONTROL 0x0 >>>> #define EXYNOS_RNG_STATUS 0x10 >>>> + >>>> +#define EXYNOS_RNG_SEED_CONF 0x14 >>>> +#define EXYNOS_RNG_GEN_PRNG 0x02 >>> >>> Use BIT(1) instead. Done. >>>> + >>>> #define EXYNOS_RNG_SEED_BASE 0x140 >>>> #define EXYNOS_RNG_SEED(n) (EXYNOS_RNG_SEED_BASE + (n * 0= x4)) >>>> #define EXYNOS_RNG_OUT_BASE 0x160 >>>> @@ -43,6 +48,11 @@ >>>> #define EXYNOS_RNG_SEED_REGS 5 >>>> #define EXYNOS_RNG_SEED_SIZE (EXYNOS_RNG_SEED_REGS * 4) >>>> >>>> +enum exynos_prng_type { >>>> + EXYNOS_PRNG_TYPE4 =3D 4, >>>> + EXYNOS_PRNG_TYPE5 =3D 5, >>> >>> That's unusual numbering and naming, so just: >>> enum exynos_prng_type { >>> EXYNOS_PRNG_EXYNOS4, >>> EXYNOS_PRNG_EXYNOS5, >>> }; >>> >>> Especially that TYPE4 and TYPE5 suggest so kind of sub-type (like >>> versions of some IP blocks, e.g. MFC) but it is just the family of >>> Exynos. >> >> Half done. I've changed TYPE to EXYNOS. >> >> I used explicit numbering in the enum because I want both values to act >> same true-false-wise. If one is 0 this condition is not met. > > First of all - that condition cannot happen. It is not possible from > the device-matching code. Let me explain what I didn't want. With the enum: enum exynos_prng_type { EXYNOS_PRNG_EXYNOS4, EXYNOS_PRNG_EXYNOS5, }; and a code like this if (rng->type) { =20=20=20=20 } EXYNOS_PRNG_EXYNOS4 is identical to false while EXYNOS_PRNG_EXYNPOS5 evaluates as true. I think this is a bad idea. I don't want it ever to happen. Because chips have their own numbers I thought using those numbers would be OK. > But if you want to indicate it explicitly > (for code reviewing?) then how about: > enum exynos_prng_type { > EXYNOS_PRNG_UNKNOWN =3D 0, > EXYNOS_PRNG_EXYNOS4, > EXYNOS_PRNG_EXYNOS5, > }; > > In such case you have the same effect but your intentions are clear > (you expect possibility of =3D0... which is not possible :) ). Fair enough. >>>> + dev_info(&pdev->dev, >>>> + "Exynos Pseudo Random Number Generator (type:= %d)\n", >>> >>> dev_dbg, this is not that important information to affect the boot time. >> >> Quite many devices report their presence during boot with such >> messages. For example: >> >> [ 3.390247] exynos-ehci 12110000.usb: EHCI Host Controller >> [ 3.395493] exynos-ehci 12110000.usb: new USB bus registered, assigne= d bus number 1 >> [ 3.403702] exynos-ehci 12110000.usb: irq 80, io mem 0x12110000 >> [ 3.431793] exynos-ehci 12110000.usb: USB 2.0 started, EHCI 1.00 >> >> From my experience it isn't printk() itself that slows down boot but the >> serial console. > > True, the console is bottleneck (not necessarily serial) [1] but that > does not change the fact there is no need to print the type of RNG. With values of the enum not being meaningful themselves printing the type does not make much sense for me too. Is it ok just to print report the device presence? =2D-=20 =C5=81ukasz Stelmach Samsung R&D Institute Poland Samsung Electronics --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBCAAGBQJaKAROAAoJELCuHpyYpYAQBQcH/115cxhZ65LJqVPtROd76qzi Hq191cpKOSA+By0HYACjEId14ZdKS+XkcpxPH//sUzNqrKf4zQ9WsxZjmNsd+Uvt vnt65fRo2QFiWQFei634/Qb2CYsHysKpejK1NNXAqFICSos6HguABYrIxUq5NYgm 4FatSoLoJ1yhrqCR8SiDX7GjXG+vktwsQc8f7TrNpP4a0LL1hvCou/gZGNeSeiEn hPdDiDTqI5sBDxTI8Sh51HXyINoAPTbwlrn/tK8KkwtEG12BJJwByhd7H3VpIPfJ sB7h+5DcvhKMFfi7D6mVkhgMHNFMeuA/6Bw7o2Rh1nvmd/0aHy0cvwxuECLAcNQ= =t5Cs -----END PGP SIGNATURE----- --=-=-=--