linux-kernel - Re: [RFC 1/5] platform-drivers-x86: add support for Technologic Systems TS-5xxx detection

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20110429233230.GB24864@kroah.com>
Date:	Fri, 29 Apr 2011 16:32:30 -0700
From:	Greg KH <greg@...ah.com>
To:	Vivien Didelot <vivien.didelot@...oirfairelinux.com>
Cc:	linux-kernel@...r.kernel.org,
	Jonas Fonseca <jonas.fonseca@...oirfairelinux.com>,
	platform-driver-x86@...r.kernel.org, linux-serial@...r.kernel.org,
	lm-sensors@...sensors.org
Subject: Re: [RFC 1/5] platform-drivers-x86: add support for Technologic
 Systems TS-5xxx detection

On Fri, Apr 29, 2011 at 06:21:48PM -0400, Vivien Didelot wrote:
> +/proc filesystem
> +----------------
> +
> +Information about the TS board is available through the /proc/ts-sbcinfo.

Really?  Why?

As you now have added a new kernel/user ABI, it must be documented in
Documentation/ABI/

Please include that in your next patch.

But first off, why a new proc file?  What is it used for?

> +static int ts_sbcinfo_init_buffer(char *buf, struct ts5xxx_sbcinfo *sbcinfo)
> +{
> +	char *pos = buf;
> +
> +	pos += ts_addbuf(pos, "Board ID", "TS-%d", sbcinfo->board_id);
> +	pos += ts_addbuf(pos, "RS485", "%s", sbcinfo->rs485 ? "yes" : "no");
> +	pos += ts_addbuf(pos, "AnalogToDigital", "%s",
> +			 sbcinfo->adc ? "yes" : "no");
> +	pos += ts_addbuf(pos, "Auto485", "%s", sbcinfo->auto485 ? "yes" : "no");
> +	pos += ts_addbuf(pos, "SRAM", "%s", sbcinfo->sram ? "yes" : "no");
> +	pos += ts_addbuf(pos, "External Reset", "%s",
> +			 sbcinfo->external_reset ? "yes" : "no");

Most of these look like they should be simple "one value per file" sysfs
files for your system.  That would make things much easier on your
userspace tools to handle parsing them properly, right?

Why not do that instead?

thanks,

greg k-h
--
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/