From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965972AbdCXP1h (ORCPT ); Fri, 24 Mar 2017 11:27:37 -0400 Received: from mailout1.samsung.com ([203.254.224.24]:52565 "EHLO mailout1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965865AbdCXP1O (ORCPT ); Fri, 24 Mar 2017 11:27:14 -0400 X-AuditID: b6c32a58-f79f16d00000132c-70-58d53abbca55 From: Bartlomiej Zolnierkiewicz To: Krzysztof Kozlowski Cc: linux-arm-kernel@lists.infradead.org, Kukjin Kim , Javier Martinez Canillas , Matt Mackall , Herbert Xu , "David S. Miller" , linux-kernel@vger.kernel.org, linux-samsung-soc@vger.kernel.org, linux-crypto@vger.kernel.org, Olof Johansson , Arnd Bergmann Subject: Re: [PATCH 1/3] crypto: hw_random - Add new Exynos RNG driver Date: Fri, 24 Mar 2017 16:26:47 +0100 Message-id: <3940941.n3qdgn1RbN@amdc3058> User-Agent: KMail/4.13.3 (Linux/3.13.0-96-generic; KDE/4.13.3; x86_64; ; ) In-reply-to: <20170324142446.31129-2-krzk@kernel.org> MIME-version: 1.0 Content-transfer-encoding: 7Bit Content-type: text/plain; charset=us-ascii X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrHKsWRmVeSWpSXmKPExsWy7bCmpu5uq6sRBpcaxSz+TjrGbjHnfAuL RfcrGYs3b9cwWfQ/fs1scf78BnaLTY+vsVrcv/eTyeLyrjlsFjPO72OyWLCtj9Hi1PXPbA48 Hr9/TWL02LLyJpPHtgOqHptWdbJ5bF5S73HlRBOrx5b+u+wefS83MHp83iQXwBnFZZOSmpNZ llqkb5fAlfFqUTtLwUShik8r7rE1MG7n62Lk5JAQMJH4tnQzG4QtJnHh3nowW0hgKaPEydvm XYxcQHY7k8TKTb0sMA33+66xQiSWM0rcmfGBEcL5yijx7eUJsCo2ASuJie2rGEFsEQFNiet/ v4N1MAtMY5bY2zCBGSQhLOAmce73I3YQm0VAVeL90Vtgu3mBGv63/QCzRQW8JLbsa2cCsTkF TCU2L1nBDlEjKPFj8j2wZcwC8hL79k9lhbB1JM4eWwd2kYTAW3aJR4/eAC3jAHJkJTYdYIZ4 wUVi2e0t7BC2sMSr4zC2tMTfpbcYIezpjBLbf0tAzNnMKLFq9wSoImuJw8cvQi3jk+j9/YQJ Yj6vREebEESJh8Sy3h/Q4HKUOLf1HxskhIDmLGu7wz6BUX4Wkh9mIflhFpIfFjAyr2IUSy0o zk1PLTYtMNErTswtLs1L10vOz93ECE5VWhE7GP/NCDrEKMDBqMTDa8F8NUKINbGsuDL3EKME B7OSCK+3KVCINyWxsiq1KD++qDQntfgQozQHi5I4b5TBxAghgfTEktTs1NSC1CKYLBMHp1QD 46yVDQuaL3x42vq0MOahXKzP5H+MDqGWvGGyajtK1qz1/7dhx85bYd+cHRprlM/pFn9w91s3 NeNv9NH6l0sfL/DiO/D6zipGRwn3102bb5/MWvni2/Hm/sADfj5WG0KvK3/tUGJ2sQ+p1GU6 VFe/T2tZrdGdZXHXK+0X/zJ32uPW015RsJefX4mlOCPRUIu5qDgRAJrbRr9RAwAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrHIsWRmVeSWpSXmKPExsVy+t9jAd1dVlcjDKbN4bH4O+kYu8Wc8y0s Ft2vZCzevF3DZNH/+DWzxfnzG9gtNj2+xmpx/95PJovLu+awWcw4v4/JYsG2PkaLU9c/sznw ePz+NYnRY8vKm0we2w6oemxa1cnmsXlJvceVE02sHlv677J79L3cwOjxeZNcAGeUm01GamJK apFCal5yfkpmXrqtUmiIm66FkkJeYm6qrVKErm9IkJJCWWJOKZBnZIAGHJwD3IOV9O0S3DJe LWpnKZgoVPFpxT22BsbtfF2MnBwSAiYS9/uusULYYhIX7q1n62Lk4hASWMooMeXEe0aQhJDA V0aJr1fCQWw2ASuJie2rwOIiApoS1/9+ZwVpYBaYxiwxd+JRsEnCAm4S534/YgexWQRUJd4f vcUGYvMCNfxv+wFmiwp4SWzZ184EYnMKmEpsXrICqJ4DaFm8RMPJdIhyQYkfk++xgNjMAvIS +/ZPZYWwtSTW7zzONIFRYBaSsllIymYhKVvAyLyKUSK1ILmgOCk91ygvtVyvODG3uDQvXS85 P3cTIzhun0nvYDy8y/0QowAHoxIP74nXVyKEWBPLiitzDzFKcDArifB6m16NEOJNSaysSi3K jy8qzUktPsRoCvTfRGYp0eR8YErJK4k3NDE3MTc2sDC3tDQxUhLnbZz9LFxIID2xJDU7NbUg tQimj4mDU6qB0XbX6b/H7nFGMMh2W8/q5Z2y8rFr3AN/h62+jw7/s10ks/exIJddyGnjir1z dc/luc+etvbddgV2sZkL3CewTKt5uOULZ7lFhcGB79a8f8x/LX857877b/WT5Tpjn7mdXtqa s237RPGyxq3L+d9Vm8158NRU6z3HTq8m7gNTpkcy6ycd25vqEqHEUpyRaKjFXFScCAA9aqp7 8QIAAA== X-MTR: 20000000000000000@CPGS X-CMS-MailID: 20170324152650epcas5p1fb48677d6b5ac7210d6df438a5e70ca3 X-Msg-Generator: CA X-Sender-IP: 203.254.230.27 X-Local-Sender: =?UTF-8?B?QmFydGxvbWllaiBab2xuaWVya2lld2ljehtTUlBPTC1LZXJu?= =?UTF-8?B?ZWwgKFRQKRvsgrzshLHsoITsnpAbU2VuaW9yIFNvZnR3YXJlIEVuZ2luZWVy?= X-Global-Sender: =?UTF-8?B?QmFydGxvbWllaiBab2xuaWVya2lld2ljehtTUlBPTC1LZXJu?= =?UTF-8?B?ZWwgKFRQKRtTYW1zdW5nIEVsZWN0cm9uaWNzG1NlbmlvciBTb2Z0d2FyZSBF?= =?UTF-8?B?bmdpbmVlcg==?= X-Sender-Code: =?UTF-8?B?QzEwG0VIURtDMTBDRDAyQ0QwMjczOTI=?= CMS-TYPE: 105P X-HopCount: 7 X-CMS-RootMailID: 20170324152650epcas5p1fb48677d6b5ac7210d6df438a5e70ca3 X-RootMTR: 20170324152650epcas5p1fb48677d6b5ac7210d6df438a5e70ca3 References: <20170324142446.31129-1-krzk@kernel.org> <20170324142446.31129-2-krzk@kernel.org> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, Firstly, thanks for working on this. The patch looks fine overall for me, some review comments below. On Friday, March 24, 2017 05:24:44 PM Krzysztof Kozlowski wrote: > Replace existing hw_ranndom/exynos-rng driver with a new, reworked one. > This is a driver for pseudo random number generator block which on > Exynos4 chipsets must be seeded with some value. On newer Exynos5420 > chipsets it might seed itself from true random number generator block > but this is not implemented yet. > > New driver is a complete rework to use the crypto ALGAPI instead of > hw_random API. Rationale for the change: > 1. hw_random interface is for true RNG devices. > 2. The old driver was seeding itself with jiffies which is not a > reliable source for randomness. > 3. Device generates five random numbers in each pass but old driver was > returning only one thus its performance was reduced. > > Compatibility with DeviceTree bindings is preserved. > > New driver does not use runtime power management but manually enables > and disables the clock when needed. This is preferred approach because > using runtime PM just to toggle clock is huge overhead. Another I'm not entirely convinced that the new approach is better. With the old approach exynos_rng_generate() can be called more than once before PM autosuspend kicks in and thus clk_prepare_enable()/ clk_disable()_unprepare() operations will be done only once. This would give better performance on the "burst" operations. [ The above assumes that clock operations are more costly than going through PM core to check the current device state. ] > +static int exynos_rng_get_random(struct exynos_rng_dev *rng, > + u8 *dst, unsigned int dlen, > + unsigned int *read) > +{ > + int retry = 100; I know that this is copied verbatim from the old driver but please use define for the maximum number of retries. > +static int exynos_rng_probe(struct platform_device *pdev) > +{ > + struct exynos_rng_dev *rng; > + struct resource *res; > + int ret; > + > + if (exynos_rng_dev) > + return -EEXIST; How this condition could ever happen? The probe function will never be called twice. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics