From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pali =?utf-8?B?Um9ow6Fy?= Subject: Re: [RESEND] [PATCH v2 1/2] arm: devtree: Set system_rev from DT revision Date: Wed, 6 May 2015 12:37:52 +0200 Message-ID: <20150506103752.GG30910@pali> References: <1430902142-17035-1-git-send-email-pali.rohar@gmail.com> <1430902142-17035-2-git-send-email-pali.rohar@gmail.com> <3577431.y872Foi4kj@wuerfel> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Content-Disposition: inline In-Reply-To: <3577431.y872Foi4kj@wuerfel> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Arnd Bergmann Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, Rob Herring , Russell King , Will Deacon , Ivaylo Dimitrov , Sebastian Reichel , Pavel Machek , Tony Lindgren , Andreas =?utf-8?Q?F=C3=A4rber?= , linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: linux-omap@vger.kernel.org On Wednesday 06 May 2015 11:31:15 Arnd Bergmann wrote: > On Wednesday 06 May 2015 10:49:01 Pali Roh=C3=A1r wrote: > > With this patch "revision" DT string entry is used to set global sy= stem_rev > > variable. DT "revision" is expected to be string with one hexadecim= al number. > > So "Revision" line in /proc/cpuinfo will be same as "revision" DT v= alue. > >=20 > > Signed-off-by: Pali Roh=C3=A1r > > Acked-by: Pavel Machek >=20 > +devicetree mailing list >=20 > The property needs to be specified in a binding somewhere. >=20 > > @@ -246,5 +247,14 @@ const struct machine_desc * __init setup_machi= ne_fdt(unsigned int dt_phys) > > /* Change machine number to match the mdesc we're using */ > > __machine_arch_type =3D mdesc->nr; > > =20 > > + /* Set system revision from DT */ > > + prop =3D 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) !=3D 0) > > + system_rev =3D 0; > > + } > > + > > return mdesc; > > } > >=20 >=20 > What is the reason for doing it this early? I think it would be nicer= to do > it after unflattening the DT. >=20 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 kstrtou= int > 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. >=20 > 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 developer= s suggested for v2 to change it to string (from number). --=20 Pali Roh=C3=A1r pali.rohar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org -- To unsubscribe from this list: send the line "unsubscribe devicetree" i= n the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 From: pali.rohar@gmail.com (Pali =?utf-8?B?Um9ow6Fy?=) Date: Wed, 6 May 2015 12:37:52 +0200 Subject: [RESEND] [PATCH v2 1/2] arm: devtree: Set system_rev from DT revision In-Reply-To: <3577431.y872Foi4kj@wuerfel> References: <1430902142-17035-1-git-send-email-pali.rohar@gmail.com> <1430902142-17035-2-git-send-email-pali.rohar@gmail.com> <3577431.y872Foi4kj@wuerfel> Message-ID: <20150506103752.GG30910@pali> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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 > > Acked-by: Pavel Machek > > +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 at gmail.com From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751978AbbEFKiN (ORCPT ); Wed, 6 May 2015 06:38:13 -0400 Received: from mail-wi0-f171.google.com ([209.85.212.171]:35507 "EHLO mail-wi0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751290AbbEFKh4 (ORCPT ); Wed, 6 May 2015 06:37:56 -0400 Date: Wed, 6 May 2015 12:37:52 +0200 From: Pali =?utf-8?B?Um9ow6Fy?= To: Arnd Bergmann Cc: linux-arm-kernel@lists.infradead.org, Rob Herring , Russell King , Will Deacon , Ivaylo Dimitrov , Sebastian Reichel , Pavel Machek , Tony Lindgren , Andreas =?utf-8?Q?F=C3=A4rber?= , linux-omap@vger.kernel.org, "linux-kernel@vger.kernel.org" , devicetree@vger.kernel.org Subject: Re: [RESEND] [PATCH v2 1/2] arm: devtree: Set system_rev from DT revision Message-ID: <20150506103752.GG30910@pali> References: <1430902142-17035-1-git-send-email-pali.rohar@gmail.com> <1430902142-17035-2-git-send-email-pali.rohar@gmail.com> <3577431.y872Foi4kj@wuerfel> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <3577431.y872Foi4kj@wuerfel> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > > Acked-by: Pavel Machek > > +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@gmail.com