On Fri, Sep 15, 2017 at 8:10 AM, Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote: > On Wed, Sep 13, 2017 at 4:24 PM, Kamil Konieczny > <k.konieczny@xxxxxxxxxxxxxxxxxxx> wrote: >> Hi Krzysztof, >> >> On 13.09.2017 15:18, Krzysztof Kozlowski wrote: >>> On Wed, Sep 13, 2017 at 2:44 PM, Kamil Konieczny >>> <k.konieczny@xxxxxxxxxxxxxxxxxxx> wrote: >>>> Add support for MD5, SHA1, SHA256 hash algorithms for Exynos HW. >>>> It uses the crypto framework asynchronous hash api. >>>> It is based on omap-sham.c driver. >>>> S5P has some HW differencies and is not implemented. >>>> >>>> Modifications in s5p-sss: >>>> >>>> - Add hash supporting structures and functions. >>>> >>>> - Modify irq handler to handle both aes and hash signals. >>>> >>>> - Disable HASH in probe if Exynos PRNG is enabled. >>>> >>>> - Add new copyright line and new author. >>>> >>>> - Tested on Odroid-U3 with Exynos 4412 CPU, kernel 4.13-rc6 >>>> with crypto run-time self test testmgr >>>> and with tcrypt module with: modprobe tcrypt sec=1 mode=N >>>> where N=402, 403, 404 (MD5, SHA1, SHA256). >>>> >>>> Modifications in drivers/crypto/Kconfig: >>>> >>>> - Select sw algorithms MD5, SHA1 and SHA256 in S5P >>>> as they are nedded for fallback. >>>> >>>> Signed-off-by: Kamil Konieczny <k.konieczny@xxxxxxxxxxxxxxxxxxx> >>>> --- >>>> drivers/crypto/Kconfig | 6 + >>>> drivers/crypto/s5p-sss.c | 2062 +++++++++++++++++++++++++++++++++++++++++++--- >>>> 2 files changed, 1939 insertions(+), 129 deletions(-) >>>> >>> >>> Nice work, thanks! >>> >>> You need to split the patch, it is just too huge making it very >>> difficult to review. Please split it per logically sensible >>> improvements. >> >> Can you suggest how to break it up ?> >> It is now big update, added working functionallity in one piece, >> but I agree it can be hard to read >> as git did some strange things, >> like delete few aes functions (and mixing this delete with '+' lines) >> and then adding the same aes without any change. > > You know the changes you want to do, you know the new architecture so > usually it is easier for you to figure out the split. > But few ideas: > 1. For the problem of functions being deleted-moved without any > changes, you can try to reorder new code so diff will not show these > hunks. Indeed a lot diff here looks like weird matching of diff/patch. > This definitely has to be fixed because I cannot figure out the real > changes around existing functions (e.g. s5p_set_indata_start(), > s5p_aes_crypt_start()). > 2. You add debugging code - FLOW_LOG - this is one candidate to split entirely. > 3. Cleanups go to separate patch. > > Probably working on (1) above will bring the most benefit. BTW, using some stable git release (not latest master) might help as well... BR, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
 |