Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1451422 > unrolled thread

Re: [PATCH 05/10] EDAC, altera: Add Arria10 NAND EDAC support

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 05/10] EDAC, altera: Add Arria10 NAND EDAC support Borislav Petkov <bp@alien8.de> - 2016-07-27 19:20 +0200
    Re: [PATCH 05/10] EDAC, altera: Add Arria10 NAND EDAC support Borislav Petkov <bp@alien8.de> - 2016-07-27 22:20 +0200

#1451422 — Re: [PATCH 05/10] EDAC, altera: Add Arria10 NAND EDAC support

On Thu, Jul 14, 2016 at 11:06:43AM -0500, tthayer@opensource.altera.com wrote:
> From: Thor Thayer <tthayer@opensource.altera.com>
> 
> Add Altera Arria10 NAND FIFO memory EDAC support.
> 
> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
> ---
>  drivers/edac/Kconfig       |    7 +++++++
>  drivers/edac/altera_edac.c |   34 +++++++++++++++++++++++++++++++++-
>  2 files changed, 40 insertions(+), 1 deletion(-)

...

> @@ -1589,7 +1619,9 @@ static int altr_edac_a10_probe(struct platform_device *pdev)
>  		else if ((of_device_is_compatible(child,
>  					"altr,socfpga-a10-ocram-ecc")) ||
>  			 (of_device_is_compatible(child,
> -					"altr,socfpga-eth-mac-ecc")))
> +					"altr,socfpga-eth-mac-ecc")) ||
> +			 (of_device_is_compatible(child,
> +						  "altr,socfpga-nand-ecc")))
>  			altr_edac_a10_device_add(edac, child);
>  		else if (of_device_is_compatible(child,
>  						 "altr,sdram-edac-a10"))

Can we simplify this loop like this?

	for_each_child_of_node(pdev->dev.of_node, child) {
		if (!of_device_is_available(child))
			continue;

		if (of_device_is_compatible(child, "altr,socfpga-a10-l2-ecc") ||
		    of_device_is_compatible(child, "altr,socfpga-a10-ocram-ecc") ||
		    of_device_is_compatible(child, "altr,socfpga-eth-mac-ecc") ||
		    of_device_is_compatible(child, "altr,socfpga-nand-ecc"))

			altr_edac_a10_device_add(edac, child);

		else if (of_device_is_compatible(child, "altr,sdram-edac-a10"))
			of_platform_populate(pdev->dev.of_node,
					     altr_sdram_ctrl_of_match,
					     NULL, &pdev->dev);
	}

I've merged the first "if" and subsequent "else if" because they all
do altr_edac_a10_device_add(edac, child) and added spacing for better
readability.

Look ok?

Or have I fatfingered it?

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

[toc] | [next] | [standalone]

#1451493

On Wed, Jul 27, 2016 at 01:43:04PM -0500, Thor Thayer wrote:
> Yes, that's better. I was trying to stay within the 80 character limit but
> missed the if/else if improvement. Thanks, Boris!
> 
> Should I re-submit?

Nah, no need.

I'd only ask you to test the final result once I've pushed it out after
the merge window is over.

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web