linux-kernel - Re: [PATCH v5 3/4] drivers: exynos-srom: Add support for bank configuration

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <563B38E9.7070600@samsung.com>
Date:	Thu, 5 Nov 2015 20:09:29 +0900
From:	Krzysztof Kozlowski <k.kozlowski@...sung.com>
To:	Pavel Fedin <p.fedin@...sung.com>, devicetree@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org,
	linux-samsung-soc@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:	k.kozlowski.k@...il.com, 'Rob Herring' <robh+dt@...nel.org>,
	'Pawel Moll' <pawel.moll@....com>,
	'Mark Rutland' <mark.rutland@....com>,
	'Ian Campbell' <ijc+devicetree@...lion.org.uk>,
	'Kumar Gala' <galak@...eaurora.org>,
	'Kukjin Kim' <kgene@...nel.org>
Subject: Re: [PATCH v5 3/4] drivers: exynos-srom: Add support for bank
 configuration

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/