Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1032315AbbKELJi (ORCPT ); Thu, 5 Nov 2015 06:09:38 -0500 Received: from mail-pa0-f52.google.com ([209.85.220.52]:35435 "EHLO mail-pa0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1032200AbbKELJg (ORCPT ); Thu, 5 Nov 2015 06:09:36 -0500 Subject: Re: [PATCH v5 3/4] drivers: exynos-srom: Add support for bank configuration To: Pavel Fedin , devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org References: <72fa025c634fa477b1d7a89c254bd3c9c43f2b27.1446542020.git.p.fedin@samsung.com> <5639BFD8.6040006@samsung.com> <004001d117b6$62baa1a0$282fe4e0$@samsung.com> Cc: k.kozlowski.k@gmail.com, "'Rob Herring'" , "'Pawel Moll'" , "'Mark Rutland'" , "'Ian Campbell'" , "'Kumar Gala'" , "'Kukjin Kim'" From: Krzysztof Kozlowski Message-ID: <563B38E9.7070600@samsung.com> Date: Thu, 5 Nov 2015 20:09:29 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <004001d117b6$62baa1a0$282fe4e0$@samsung.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1745 Lines: 59 W dniu 05.11.2015 o 19:40, Pavel Fedin pisze: > Hello! > >>> +static int decode_sromc(struct exynos_srom *srom, struct device_node *np) >> >> I missed that one previously: add prefix and more descriptive name, like: >> exynos_srom_parse_child() > > exynos_srom_configure_bank(), is this name OK? Yes, its OK. > >>> static int exynos_srom_probe(struct platform_device *pdev) >>> { >>> - struct device_node *np; >>> + struct device_node *np, *child; >>> struct exynos_srom *srom; >>> struct device *dev = &pdev->dev; >>> + bool error = false; >> >> The 'error' name is misleading - like error for entire probe which is >> not true. >> >> Instead split it to separate function like: >> >> +static int exynos_srom_parse_children(....) { >> + int ret = 0; >> + >> + for_each_child_of_node(np, child) { >> + ret = exynos_srom_parse_child(srom, child); >> + if (ret) { >> + dev_err(dev, >> + "Could not decode bank configuration for %s: %d\n", >> + child->name, ret); >> + break; >> + } >> + } >> + >> + return ret; >> +} > > Factoring out this loop is unnecessary, because i could just 'return 0' in the loop > instead of 'error = true'. Byt my idea is to go through all banks anyway, just in > case, to diagnose all of them. So that the user will be able to spot and fix all > broken banks at once, instead of doing one-by-one. > I have renamed the variable to 'bool bad_bank_config', will this be OK? Yes, that's also OK. Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/