[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 2/2] fdt: make fdt handling reusable across arch


  • To: "Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>
  • From: Luca Fancellu <Luca.Fancellu@xxxxxxx>
  • Date: Thu, 3 Aug 2023 18:05:52 +0000
  • Accept-language: en-GB, en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=E0NtMGP5Ni8N6PrPUiOBVIbo4Ed4wZldRchR3+VrtTE=; b=ZGVv2yjOBsmvKkDhIMcEa7urmMTNMMUqCNCjliQv4hua1xaknrFVEDCCdrpXuk42m33brLXSYtZNyxRUfJ9R47vUm7S91hcV/fLU9nb90b6PcqHiRGgMjU6YvIJD+ayLs5U7JMZkOtdUYMDtvw4mXcRFmbe9gKjEwQ1xeNMfPPtQVyT53vJKlP7FFO0oMikAcF+JVn8WXn4hwtjmwFu6YPuJHuJXbHP/4qzEyW+FpSwfKi0JQ2O/t2/kYQnlnqHWE/+7OFbGIZPbTLwK+tP/6SlK3jKNPG7bAKWzMlIN+JgL7KYsWwEvKnLQrDaLK69ObAzfuagxYoRaGVme6ZDZNQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=AwqhfP3G9aRsrME8GbCft44S5za4Apnwfd3qqf7aETi+XpYhl9me9grfj4fsLMDzbi4cID4sTft80c7qR0wcxx++kixfIp9jHj/5uI2DDPLjP+iKWfNprkMfgRjgE5iOEImQ/fOM37I88Nd+zKhj28du2VbW9SApeVD1aS+51+mJiiU0wF+DrLdexropt5u+fxdpHdBWljJDmmfzJrANmBZGxISiewKn0gtD0Ut8f84P9XYLCxxkxHtVcLFNnLQ0bWo6HxxoWENUvBGCR/P3GzikimykauYvJNW+RqOMy/+JS4e2WJtADl/2XGzTwCWREV1XYhkRS8xED168a3PNfA==
  • Authentication-results-original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Cc: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
  • Delivery-date: Thu, 03 Aug 2023 18:07:08 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHZxfetD3iMBVO78kiKHyDm3xOwK6/Y3kwA
  • Thread-topic: [PATCH v2 2/2] fdt: make fdt handling reusable across arch

Hi Daniel,

[...]
> diff --git a/xen/common/fdt.c b/xen/common/fdt.c
> new file mode 100644
> index 0000000000..8d7acaaa43
> --- /dev/null
> +++ b/xen/common/fdt.c
> @@ -0,0 +1,153 @@
> +/*
> + * Flattened Device Tree
> + *
> + * Copyright (C) 2012-2014 Citrix Systems, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */

Can you add an empty line here, I think it improves readability, I know that 
some other
headers don’t add it unfortunately

> +#include <xen/fdt.h>
> +#include <xen/init.h>
> +#include <xen/lib.h>
> +#include <xen/libfdt/libfdt.h>
> +#include <xen/types.h>
> +
> +bool __init device_tree_node_matches(
> +    const void *fdt, int node, const char *match)
> +{
[...]

Empty line

> +#ifndef __XEN_FDT_H__
> +#define __XEN_FDT_H__
> +
> +#include <xen/init.h>
> +#include <xen/libfdt/libfdt.h>
> +#include <xen/types.h>

For the new files, apart from Michal’s comments, if I remember correctly in the 
past I was asked
to add these lines to the end of the file:

/*
* Local variables:
* mode: C
* c-file-style: "BSD"
* c-basic-offset: 4
* indent-tabs-mode: nil
* End:
*/

Regarding the coding style, I think it’s better to keep the style you’ve found 
in the original file,
and change only some bits when the code is not following it.

I know there is nothing enforcing parameters on the same line of the function 
definition at the
moment, but it is how it’s done from the original file so I would stick with it.

Regarding the u32/u64 types, maybe since you are moving the code it can be the 
occasion to
convert them, but check with the maintainer before.

I’ve also tested this serie and it works fine! I’m not leaving any tag because 
this patch is going to
change anyway.

Cheers,
Luca


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.