linux-kernel - Re: [PATCH v3 3/3] documentation: misc: intel_sysid: Add the system id sysfs documentation for Altera(Intel) FPGA platform

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]

Date:   Thu, 28 Jul 2022 09:51:19 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     kah.jing.lee@...el.com
Cc:     linux-kernel@...r.kernel.org, arnd@...db.de,
        rafael.j.wysocki@...el.com, tien.sung.ang@...el.com,
        dinh.nguyen@...el.com, Zhou Furong <furong.zhou@...el.com>,
        Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
Subject: Re: [PATCH v3 3/3] documentation: misc: intel_sysid: Add the system
 id sysfs documentation for Altera(Intel) FPGA platform

On Mon, Jul 25, 2022 at 11:59:41AM +0800, kah.jing.lee@...el.com wrote:
> From: Kah Jing Lee <kah.jing.lee@...el.com>
> 
> This sysfs documentation is created for Altera(Intel) FPGA platform
> System ID soft IP. The Altera(Intel) Sysid component is generally
> part of an FPGA design.
> The component can be hotplugged when the FPGA is reconfigured.
> 
> Based on an initial contribution from Ley Foon Tan at Altera
> Signed-off-by: Kah Jing Lee <kah.jing.lee@...el.com>

You always need a blank line before the signed-off-by, didn't checkpatch
complain about this?

> Reviewed-by: Zhou Furong <furong.zhou@...el.com>
> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
> 
> ---
> v2->v3:
> - Updated kernel version & date
> - Added format for sysid & builtime output

Please resend patches as whole new series, not as odd responses to
responses like this as it is impossible to figure out this way.

Please take some time and review patches from others to see just how
this process works.

> ---
> ---
>  .../testing/sysfs-devices-platform-soc-sysid  | 41 +++++++++++++++++++
>  1 file changed, 41 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-devices-platform-soc-sysid
> 
> diff --git a/Documentation/ABI/testing/sysfs-devices-platform-soc-sysid b/Documentation/ABI/testing/sysfs-devices-platform-soc-sysid
> new file mode 100644
> index 000000000000..6e40d154601f
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-devices-platform-soc-sysid
> @@ -0,0 +1,41 @@
> +What:		/sys/devices/platform/soc@...oc:base_fpga_region/
> +soc:base_fpga_region:fpga_pr_region0/XXXXXXXX.sysid/
> +Date:		July 2022

July is almost over.

> +KernelVersion:	v5.20
> +Contact:	Kah Jing Lee <kah.jing.lee@...el.com>
> +Description:
> +		The soc@...oc:base_fpga_region/soc:base_fpga_region:fpga_pr_region0/
> +		XXXXXXXX.sysid/ directory contains read-only attributes exposing
> +		information about an System ID soft IP device. The X values could vary,
> +		based on the FPGA platform System ID soft IP register address.
> +
> +What:		.../XXXXXXX.sysid/sysid
> +Date:		July 2022
> +KernelVersion:	v5.20
> +Contact:	Kah Jing Lee <kah.jing.lee@...el.com>
> +Description:
> +		The .../XXXXXXX.sysid/sysid file contains the System ID for the FPGA
> +		platform which is unique for the platform type and can be used for
> +		checking the platform type for software download purposes. Sysid value
> +		reported is in numerical format, and can also be printed in hex format
> +		for human-readable string.
> +		E.g:
> +			root@...lex:~# cat /sys/bus/platform/drivers/altera_sysid/
> +				f9000900.sysid/sysid/id

Why the line wrapping?

> +			4207856382
> +			root@...lex:~# cat /sys/bus/platform/drivers/altera_sysid/
> +				f9000900.sysid/sysid/id | xargs printf "0x%08x\n"
> +			0xfacecafe

If userspace wants this in hex format, just have the sysfs file export a
hex format.  No need to provide a tutorial for how to convert bases in
userspace, right?

> +
> +What:		.../XXXXXXX.sysid/buildtime
> +Date:		July 2022
> +KernelVersion:	v5.20
> +Contact:	Kah Jing Lee <kah.jing.lee@...el.com>
> +Description:
> +		The .../XXXXXXX.sysid/buildtime file contains the buildtime for the
> +		FPGA board file generation. Buildtime value reported in
> +		<Unix timestamp> (YYYY-mm-dd HH:MM:SS UTC) format.
> +		E.g:
> +			root@...lex:~# cat /sys/bus/platform/drivers/altera_sysid/
> +				f9000900.sysid/sysid/timestamp
> +			1637751409 (2021-11-24 10:56:49 UTC)

Why not use the proper RFC format for time here instead of making up a
new one?  And who will use this file, why does it have to be parsed like
this?

thanks,

greg k-h