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:	Wed, 6 May 2015 12:37:52 +0200
From:	Pali Rohár <pali.rohar@...il.com>
To:	Arnd Bergmann <arnd@...db.de>
Cc:	linux-arm-kernel@...ts.infradead.org,
	Rob Herring <robherring2@...il.com>,
	Russell King <linux@....linux.org.uk>,
	Will Deacon <will.deacon@....com>,
	Ivaylo Dimitrov <ivo.g.dimitrov.75@...il.com>,
	Sebastian Reichel <sre@...ian.org>,
	Pavel Machek <pavel@....cz>, Tony Lindgren <tony@...mide.com>,
	Andreas Färber <afaerber@...e.de>,
	linux-omap@...r.kernel.org,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	devicetree@...r.kernel.org
Subject: Re: [RESEND] [PATCH v2 1/2] arm: devtree: Set system_rev from DT
 revision

On Wednesday 06 May 2015 11:31:15 Arnd Bergmann wrote:
> On Wednesday 06 May 2015 10:49:01 Pali Rohár wrote:
> > With this patch "revision" DT string entry is used to set global system_rev
> > variable. DT "revision" is expected to be string with one hexadecimal number.
> > So "Revision" line in /proc/cpuinfo will be same as "revision" DT value.
> > 
> > Signed-off-by: Pali Rohár <pali.rohar@...il.com>
> > Acked-by: Pavel Machek <pavel@....cz>
> 
> +devicetree mailing list
> 
> The property needs to be specified in a binding somewhere.
> 
> > @@ -246,5 +247,14 @@ const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
> >  	/* Change machine number to match the mdesc we're using */
> >  	__machine_arch_type = mdesc->nr;
> >  
> > +	/* Set system revision from DT */
> > +	prop = of_get_flat_dt_prop(dt_root, "revision", &size);
> > +	if (prop && size > 0) {
> > +		char revision[11];
> > +		strlcpy(revision, prop, min(size, (int)sizeof(revision)));
> > +		if (kstrtouint(revision, 16, &system_rev) != 0)
> > +			system_rev = 0;
> > +	}
> > +
> >  	return mdesc;
> >  }
> > 
> 
> What is the reason for doing it this early? I think it would be nicer to do
> it after unflattening the DT.
> 

It needs to be done in this code, so "system_rev" variable is set
properly...

> Also, it seems strange to have a string property and then use kstrtouint
> to convert it into a number. I think it should either be specified in a DT
> binding to be a string and then have the kernel not assume that it is a number,
> or we should define it to be binary.
> 
> 	Arnd

Variable "system_rev" is number and it always was. So chaning type will
break more parts.

And it is string DT property to be human readable. Some other developers
suggested for v2 to change it to string (from number).

-- 
Pali Rohár
pali.rohar@...il.com
--
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/

Powered by blists - more mailing lists