Re: [RESEND] [PATCH v2 1/2] arm: devtree: Set system

devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* Re: [RESEND] [PATCH v2 1/2] arm: devtree: Set system_rev from DT revision
       [not found] ` <1430902142-17035-2-git-send-email-pali.rohar@gmail.com>
@ 2015-05-06  9:31   ` Arnd Bergmann
  2015-05-06 10:37     ` Pali Rohár
  0 siblings, 1 reply; 20+ messages in thread
From: Arnd Bergmann @ 2015-05-06  9:31 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Pali Rohár, Rob Herring, Russell King, Will Deacon,
	Ivaylo Dimitrov, Sebastian Reichel, Pavel Machek, Tony Lindgren,
	Andreas Färber, linux-omap, linux-kernel@vger.kernel.org,
	devicetree

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@gmail.com>
> Acked-by: Pavel Machek <pavel@ucw.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.

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RESEND] [PATCH v2 1/2] arm: devtree: Set system_rev from DT revision
  2015-05-06  9:31   ` [RESEND] [PATCH v2 1/2] arm: devtree: Set system_rev from DT revision Arnd Bergmann
@ 2015-05-06 10:37     ` Pali Rohár
  2015-05-06 11:04       ` Arnd Bergmann
  0 siblings, 1 reply; 20+ messages in thread
From: Pali Rohár @ 2015-05-06 10:37 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Rob Herring,
	Russell King, Will Deacon, Ivaylo Dimitrov, Sebastian Reichel,
	Pavel Machek, Tony Lindgren, Andreas Färber,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA

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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
> 
> +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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RESEND] [PATCH v2 1/2] arm: devtree: Set system_rev from DT revision
  2015-05-06 10:37     ` Pali Rohár
@ 2015-05-06 11:04       ` Arnd Bergmann
  2015-05-06 11:44         ` Pali Rohár
  0 siblings, 1 reply; 20+ messages in thread
From: Arnd Bergmann @ 2015-05-06 11:04 UTC (permalink / raw)
  To: Pali Rohár
  Cc: linux-arm-kernel, Rob Herring, Russell King, Will Deacon,
	Ivaylo Dimitrov, Sebastian Reichel, Pavel Machek, Tony Lindgren,
	Andreas Färber, linux-omap, linux-kernel@vger.kernel.org,
	devicetree

On Wednesday 06 May 2015 12:37:52 Pali Rohár wrote:
> 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@gmail.com>
> > > Acked-by: Pavel Machek <pavel@ucw.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...

What I mean is which code accesses this variable that early?

> > 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).

Both of them would be human readable, you just use something else to
read them ;-)

If we have a string here, we should just change all uses of system_rev
in the kernel accordingly, there are only a few of them:

$ git grep -w system_rev
arch/arm/include/asm/system_info.h:extern unsigned int system_rev;
arch/arm/kernel/atags_parse.c:  system_rev = tag->u.revision.rev;
arch/arm/kernel/setup.c:unsigned int system_rev;
arch/arm/kernel/setup.c:EXPORT_SYMBOL(system_rev);
arch/arm/kernel/setup.c:        seq_printf(m, "Revision\t: %04x\n", system_rev);
arch/arm/mach-clps711x/devices.c:       system_rev = SYSFLG1_VERID(readl(base + SYSFLG1));
arch/arm/mach-clps711x/devices.c:       soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%u", system_rev);
arch/arm/mach-davinci/board-da850-evm.c:        switch (system_rev & 0xF) {
arch/arm/mach-imx/mach-imx27_visstrim_m10.c:    system_rev = 0x27000;
arch/arm/mach-imx/mach-imx27_visstrim_m10.c:    system_rev |= (mo_version << MOTHERBOARD_SHIFT);
arch/arm/mach-imx/mach-imx27_visstrim_m10.c:    system_rev |= (exp_version << EXPBOARD_SHIFT);
arch/arm/mach-imx/mach-imx27_visstrim_m10.c:    mo_version = (system_rev >> MOTHERBOARD_SHIFT) & VERSION_MASK;
arch/arm/mach-ixp4xx/goramo_mlr.c:              system_rev = __raw_readl(flash + CFG_REV);
arch/arm/mach-omap2/board-rx51-peripherals.c:   if ((system_rev >= SYSTEM_REV_S_USES_VAUX3 && system_rev < 0x100) ||
arch/arm/mach-omap2/board-rx51-peripherals.c:       system_rev >= SYSTEM_REV_B_USES_VAUX3) {
arch/arm/mach-orion5x/dns323-setup.c:   if (machine_is_dns323() && system_rev == DNS323_REV_A1)
arch/arm/mach-orion5x/dns323-setup.c:   system_rev = dns323_identify_rev();
arch/arm/mach-orion5x/dns323-setup.c:   pr_info("DNS-323: Identified HW revision %c1\n", 'A' + system_rev);
arch/arm/mach-orion5x/dns323-setup.c:   switch(system_rev) {
arch/arm/mach-orion5x/dns323-setup.c:   switch(system_rev) {
arch/arm/mach-orion5x/dns323-setup.c:   switch(system_rev) {
arch/arm/mach-pxa/cm-x300.c:    if (system_rev < 130)
arch/arm/mach-pxa/cm-x300.c:    if (system_rev < 130) {
arch/arm/mach-pxa/cm-x300.c:    if (system_rev < 130)
arch/arm/mach-pxa/magician.c:           if (system_rev < 3)
arch/arm/mach-pxa/magician.c:           if (system_rev < 3)
arch/arm/mach-pxa/magician.c:           system_rev = board_id & 0x7;
arch/arm/mach-pxa/magician.c:           if (lcd_select && (system_rev < 3))
arch/arm/mach-pxa/raumfeld.c:   if ((system_rev & 0xff) == 2) {
arch/arm/mach-pxa/raumfeld.c:   if ((system_rev & 0xff) > 1) {
arch/arm/mach-pxa/viper.c:              system_rev = (VIPER_BOARD_VERSION(version) << 8) |
arch/arm/mach-pxa/zeus.c:       system_rev = __raw_readw(ZEUS_CPLD_VERSION);
arch/arm/mach-pxa/zeus.c:       pr_info("Zeus CPLD V%dI%d\n", (system_rev & 0xf0) >> 4, (system_rev & 0x0f));
arch/arm/mach-zynq/common.c:    system_rev = zynq_get_revision();
arch/arm/mach-zynq/common.c:    soc_dev_attr->revision = kasprintf(GFP_KERNEL, "0x%x", system_rev);

In fact, half the uses of this actually assign the revision number themselves.
code outside of arch/arm/mach-* and /proc/cpuinfo currently uses the variable.

	Arnd

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RESEND] [PATCH v2 1/2] arm: devtree: Set system_rev from DT revision
  2015-05-06 11:04       ` Arnd Bergmann
@ 2015-05-06 11:44         ` Pali Rohár
  2015-06-25  5:01           ` Tony Lindgren
  2015-06-25 10:03           ` Russell King - ARM Linux
  0 siblings, 2 replies; 20+ messages in thread
From: Pali Rohár @ 2015-05-06 11:44 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Rob Herring,
	Russell King, Will Deacon, Ivaylo Dimitrov, Sebastian Reichel,
	Pavel Machek, Tony Lindgren, Andreas Färber,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA

On Wednesday 06 May 2015 13:04:01 Arnd Bergmann wrote:
> On Wednesday 06 May 2015 12:37:52 Pali Rohár wrote:
> > 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > > Acked-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
> > > 
> > > +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...
> 
> What I mean is which code accesses this variable that early?
> 

ATAG code is doing it at same early stage, so I added it to same early
stage...

> > > 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).
> 
> Both of them would be human readable, you just use something else to
> read them ;-)
> 
> If we have a string here, we should just change all uses of system_rev
> in the kernel accordingly, there are only a few of them:
> 
> $ git grep -w system_rev
> arch/arm/include/asm/system_info.h:extern unsigned int system_rev;
> arch/arm/kernel/atags_parse.c:  system_rev = tag->u.revision.rev;
> arch/arm/kernel/setup.c:unsigned int system_rev;
> arch/arm/kernel/setup.c:EXPORT_SYMBOL(system_rev);
> arch/arm/kernel/setup.c:        seq_printf(m, "Revision\t: %04x\n", system_rev);
> arch/arm/mach-clps711x/devices.c:       system_rev = SYSFLG1_VERID(readl(base + SYSFLG1));
> arch/arm/mach-clps711x/devices.c:       soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%u", system_rev);
> arch/arm/mach-davinci/board-da850-evm.c:        switch (system_rev & 0xF) {
> arch/arm/mach-imx/mach-imx27_visstrim_m10.c:    system_rev = 0x27000;
> arch/arm/mach-imx/mach-imx27_visstrim_m10.c:    system_rev |= (mo_version << MOTHERBOARD_SHIFT);
> arch/arm/mach-imx/mach-imx27_visstrim_m10.c:    system_rev |= (exp_version << EXPBOARD_SHIFT);
> arch/arm/mach-imx/mach-imx27_visstrim_m10.c:    mo_version = (system_rev >> MOTHERBOARD_SHIFT) & VERSION_MASK;
> arch/arm/mach-ixp4xx/goramo_mlr.c:              system_rev = __raw_readl(flash + CFG_REV);
> arch/arm/mach-omap2/board-rx51-peripherals.c:   if ((system_rev >= SYSTEM_REV_S_USES_VAUX3 && system_rev < 0x100) ||
> arch/arm/mach-omap2/board-rx51-peripherals.c:       system_rev >= SYSTEM_REV_B_USES_VAUX3) {
> arch/arm/mach-orion5x/dns323-setup.c:   if (machine_is_dns323() && system_rev == DNS323_REV_A1)
> arch/arm/mach-orion5x/dns323-setup.c:   system_rev = dns323_identify_rev();
> arch/arm/mach-orion5x/dns323-setup.c:   pr_info("DNS-323: Identified HW revision %c1\n", 'A' + system_rev);
> arch/arm/mach-orion5x/dns323-setup.c:   switch(system_rev) {
> arch/arm/mach-orion5x/dns323-setup.c:   switch(system_rev) {
> arch/arm/mach-orion5x/dns323-setup.c:   switch(system_rev) {
> arch/arm/mach-pxa/cm-x300.c:    if (system_rev < 130)
> arch/arm/mach-pxa/cm-x300.c:    if (system_rev < 130) {
> arch/arm/mach-pxa/cm-x300.c:    if (system_rev < 130)
> arch/arm/mach-pxa/magician.c:           if (system_rev < 3)
> arch/arm/mach-pxa/magician.c:           if (system_rev < 3)
> arch/arm/mach-pxa/magician.c:           system_rev = board_id & 0x7;
> arch/arm/mach-pxa/magician.c:           if (lcd_select && (system_rev < 3))
> arch/arm/mach-pxa/raumfeld.c:   if ((system_rev & 0xff) == 2) {
> arch/arm/mach-pxa/raumfeld.c:   if ((system_rev & 0xff) > 1) {
> arch/arm/mach-pxa/viper.c:              system_rev = (VIPER_BOARD_VERSION(version) << 8) |
> arch/arm/mach-pxa/zeus.c:       system_rev = __raw_readw(ZEUS_CPLD_VERSION);
> arch/arm/mach-pxa/zeus.c:       pr_info("Zeus CPLD V%dI%d\n", (system_rev & 0xf0) >> 4, (system_rev & 0x0f));
> arch/arm/mach-zynq/common.c:    system_rev = zynq_get_revision();
> arch/arm/mach-zynq/common.c:    soc_dev_attr->revision = kasprintf(GFP_KERNEL, "0x%x", system_rev);
> 
> In fact, half the uses of this actually assign the revision number themselves.
> code outside of arch/arm/mach-* and /proc/cpuinfo currently uses the variable.
> 
> 	Arnd

-- 
Pali Rohár
pali.rohar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RESEND] [PATCH v2 1/2] arm: devtree: Set system_rev from DT revision
  2015-05-06 11:44         ` Pali Rohár
@ 2015-06-25  5:01           ` Tony Lindgren
  2015-06-25  7:18             ` Pali Rohár
       [not found]             ` <20150625050138.GL4156-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
  2015-06-25 10:03           ` Russell King - ARM Linux
  1 sibling, 2 replies; 20+ messages in thread
From: Tony Lindgren @ 2015-06-25  5:01 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Arnd Bergmann, linux-arm-kernel, Rob Herring, Russell King,
	Will Deacon, Ivaylo Dimitrov, Sebastian Reichel, Pavel Machek,
	Andreas Färber, linux-omap, linux-kernel@vger.kernel.org,
	devicetree

* Pali Rohár <pali.rohar@gmail.com> [150506 04:45]:
> On Wednesday 06 May 2015 13:04:01 Arnd Bergmann wrote:
> > > 
> > > It needs to be done in this code, so "system_rev" variable is set
> > > properly...
> > 
> > What I mean is which code accesses this variable that early?
> > 
> 
> ATAG code is doing it at same early stage, so I added it to same early
> stage...

Yes we should do this early like the other atags.
 
> > > > 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).
> > 
> > Both of them would be human readable, you just use something else to
> > read them ;-)
> > 
> > If we have a string here, we should just change all uses of system_rev
> > in the kernel accordingly, there are only a few of them:

Let's just keep it as a hex as it was. After all it's an existing
interface in /proc that user space programs may expect to be in
hex format already.

Pali, care to repost the whole set again right after -rc1 with
with rev property naming and documentation added? Just keep it
as hex and let's forget any string conversion.

Regards,

Tony

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RESEND] [PATCH v2 1/2] arm: devtree: Set system_rev from DT revision
  2015-06-25  5:01           ` Tony Lindgren
@ 2015-06-25  7:18             ` Pali Rohár
  2015-06-25  7:22               ` Tony Lindgren
       [not found]             ` <20150625050138.GL4156-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
  1 sibling, 1 reply; 20+ messages in thread
From: Pali Rohár @ 2015-06-25  7:18 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Arnd Bergmann, linux-arm-kernel, Rob Herring, Russell King,
	Will Deacon, Ivaylo Dimitrov, Sebastian Reichel, Pavel Machek,
	Andreas Färber, linux-omap, linux-kernel@vger.kernel.org,
	devicetree

On Wednesday 24 June 2015 22:01:38 Tony Lindgren wrote:
> * Pali Rohár <pali.rohar@gmail.com> [150506 04:45]:
> > On Wednesday 06 May 2015 13:04:01 Arnd Bergmann wrote:
> > > > 
> > > > It needs to be done in this code, so "system_rev" variable is set
> > > > properly...
> > > 
> > > What I mean is which code accesses this variable that early?
> > > 
> > 
> > ATAG code is doing it at same early stage, so I added it to same early
> > stage...
> 
> Yes we should do this early like the other atags.
>  
> > > > > 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).
> > > 
> > > Both of them would be human readable, you just use something else to
> > > read them ;-)
> > > 
> > > If we have a string here, we should just change all uses of system_rev
> > > in the kernel accordingly, there are only a few of them:
> 
> Let's just keep it as a hex as it was. After all it's an existing
> interface in /proc that user space programs may expect to be in
> hex format already.
> 
> Pali, care to repost the whole set again right after -rc1 with
> with rev property naming and documentation added? Just keep it
> as hex and let's forget any string conversion.
> 
> Regards,
> 
> Tony

Ok, but what do you mean to forget any string conversion?

-- 
Pali Rohár
pali.rohar@gmail.com

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RESEND] [PATCH v2 1/2] arm: devtree: Set system_rev from DT revision
  2015-06-25  7:18             ` Pali Rohár
@ 2015-06-25  7:22               ` Tony Lindgren
  2015-06-25  7:27                 ` Pali Rohár
  0 siblings, 1 reply; 20+ messages in thread
From: Tony Lindgren @ 2015-06-25  7:22 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Arnd Bergmann, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Rob Herring, Russell King, Will Deacon, Ivaylo Dimitrov,
	Sebastian Reichel, Pavel Machek, Andreas Färber,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA

* Pali Rohár <pali.rohar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> [150625 00:21]:
> 
> Ok, but what do you mean to forget any string conversion?

No need for tohexstr() in the uncommpress code if the system_rev
value is a number coming from the dts.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RESEND] [PATCH v2 1/2] arm: devtree: Set system_rev from DT revision
  2015-06-25  7:22               ` Tony Lindgren
@ 2015-06-25  7:27                 ` Pali Rohár
  2015-06-25  7:41                   ` Tony Lindgren
  0 siblings, 1 reply; 20+ messages in thread
From: Pali Rohár @ 2015-06-25  7:27 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Arnd Bergmann, linux-arm-kernel, Rob Herring, Russell King,
	Will Deacon, Ivaylo Dimitrov, Sebastian Reichel, Pavel Machek,
	Andreas Färber, linux-omap, linux-kernel@vger.kernel.org,
	devicetree

On Thursday 25 June 2015 00:22:05 Tony Lindgren wrote:
> * Pali Rohár <pali.rohar@gmail.com> [150625 00:21]:
> > 
> > Ok, but what do you mean to forget any string conversion?
> 
> No need for tohexstr() in the uncommpress code if the system_rev
> value is a number coming from the dts.
> 
> Regards,
> 
> Tony

So /revision DT property will be (binary) value, right?

-- 
Pali Rohár
pali.rohar@gmail.com

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RESEND] [PATCH v2 1/2] arm: devtree: Set system_rev from DT revision
  2015-06-25  7:27                 ` Pali Rohár
@ 2015-06-25  7:41                   ` Tony Lindgren
  0 siblings, 0 replies; 20+ messages in thread
From: Tony Lindgren @ 2015-06-25  7:41 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Arnd Bergmann, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Rob Herring, Russell King, Will Deacon, Ivaylo Dimitrov,
	Sebastian Reichel, Pavel Machek, Andreas Färber,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA

* Pali Rohár <pali.rohar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> [150625 00:29]:
> On Thursday 25 June 2015 00:22:05 Tony Lindgren wrote:
> > * Pali Rohár <pali.rohar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> [150625 00:21]:
> > > 
> > > Ok, but what do you mean to forget any string conversion?
> > 
> > No need for tohexstr() in the uncommpress code if the system_rev
> > value is a number coming from the dts.
> 
> So /revision DT property will be (binary) value, right?

Right just u32 value.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 20+ messages in thread

[parent not found: <20150625050138.GL4156-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>]

* Re: [RESEND] [PATCH v2 1/2] arm: devtree: Set system_rev from DT revision
       [not found]             ` <20150625050138.GL4156-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
@ 2015-07-06 12:23               ` Pali Rohár
  2015-07-06 12:31                 ` Tony Lindgren
  0 siblings, 1 reply; 20+ messages in thread
From: Pali Rohár @ 2015-07-06 12:23 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Arnd Bergmann, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Rob Herring, Russell King, Will Deacon, Ivaylo Dimitrov,
	Sebastian Reichel, Pavel Machek, Andreas Färber,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: Text/Plain, Size: 1965 bytes --]

On Thursday 25 June 2015 07:01:38 Tony Lindgren wrote:
> * Pali Rohár <pali.rohar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> [150506 04:45]:
> > On Wednesday 06 May 2015 13:04:01 Arnd Bergmann wrote:
> > > > It needs to be done in this code, so "system_rev" variable is
> > > > set properly...
> > > 
> > > What I mean is which code accesses this variable that early?
> > 
> > ATAG code is doing it at same early stage, so I added it to same
> > early stage...
> 
> Yes we should do this early like the other atags.
> 
> > > > > 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).
> > > 
> > > Both of them would be human readable, you just use something else
> > > to read them ;-)
> > > 
> > > If we have a string here, we should just change all uses of
> > > system_rev
> 
> > > in the kernel accordingly, there are only a few of them:
> Let's just keep it as a hex as it was. After all it's an existing
> interface in /proc that user space programs may expect to be in
> hex format already.
> 
> Pali, care to repost the whole set again right after -rc1 with
> with rev property naming and documentation added? Just keep it
> as hex and let's forget any string conversion.
> 
> Regards,
> 
> Tony

Hello Tony,

into which file should I put documentation about new DT properties?

-- 
Pali Rohár
pali.rohar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RESEND] [PATCH v2 1/2] arm: devtree: Set system_rev from DT revision
  2015-07-06 12:23               ` Pali Rohár
@ 2015-07-06 12:31                 ` Tony Lindgren
       [not found]                   ` <20150706123126.GG10705-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
  2015-07-06 15:20                   ` Rob Herring
  0 siblings, 2 replies; 20+ messages in thread
From: Tony Lindgren @ 2015-07-06 12:31 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Arnd Bergmann, linux-arm-kernel, Rob Herring, Russell King,
	Will Deacon, Ivaylo Dimitrov, Sebastian Reichel, Pavel Machek,
	Andreas Färber, linux-omap, linux-kernel@vger.kernel.org,
	devicetree

* Pali Rohár <pali.rohar@gmail.com> [150706 05:25]:
>
> into which file should I put documentation about new DT properties?

If it's Linux generic like linux,revision, then how about
Documentation/devicetree/bindings/revision.txt?

For the ATAGs, Documentation/devicetree/bindings/arm/atag.txt?

Regards,

Tony

^ permalink raw reply	[flat|nested] 20+ messages in thread

[parent not found: <20150706123126.GG10705-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>]

* Re: [RESEND] [PATCH v2 1/2] arm: devtree: Set system_rev from DT revision
       [not found]                   ` <20150706123126.GG10705-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
@ 2015-07-06 13:12                     ` Pali Rohár
  2015-07-06 13:55                       ` Tony Lindgren
  2015-07-06 15:22                       ` Rob Herring
  0 siblings, 2 replies; 20+ messages in thread
From: Pali Rohár @ 2015-07-06 13:12 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Arnd Bergmann, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Rob Herring, Russell King, Will Deacon, Ivaylo Dimitrov,
	Sebastian Reichel, Pavel Machek, Andreas Färber,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: Text/Plain, Size: 1100 bytes --]

On Monday 06 July 2015 14:31:27 Tony Lindgren wrote:
> * Pali Rohár <pali.rohar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> [150706 05:25]:
> > into which file should I put documentation about new DT properties?
> 
> If it's Linux generic like linux,revision, then how about
> Documentation/devicetree/bindings/revision.txt?
> 
> For the ATAGs, Documentation/devicetree/bindings/arm/atag.txt?
> 
> Regards,
> 
> Tony

Hm... now I'm thinking into which DT field should I put atags and 
revision. In previous emails you wrote to use "linux,atags", now 
"arm,atags"? And put them into root node? Or other?

In arch/arm/boot/compressed/atags_to_fdt.c code I see that most atag 
properties are converted into "/chosen" node in DT...

So what do you prefer for "revision" and what for "atags"?

Some mentioned examples:

"/revision"
"/chosen/revision"
"/linux,revision"
"/chosen/linux,revision"
...

"/atags"
"/chosen/atags"
"/linux,atags"
"/chosen/linux,atags"
"/arm,atags"
"/chosen/arm,atags"
...

-- 
Pali Rohár
pali.rohar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RESEND] [PATCH v2 1/2] arm: devtree: Set system_rev from DT revision
  2015-07-06 13:12                     ` Pali Rohár
@ 2015-07-06 13:55                       ` Tony Lindgren
  2015-07-06 15:22                       ` Rob Herring
  1 sibling, 0 replies; 20+ messages in thread
From: Tony Lindgren @ 2015-07-06 13:55 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Arnd Bergmann, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Rob Herring, Russell King, Will Deacon, Ivaylo Dimitrov,
	Sebastian Reichel, Pavel Machek, Andreas Färber,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA

* Pali Rohár <pali.rohar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> [150706 06:14]:
> On Monday 06 July 2015 14:31:27 Tony Lindgren wrote:
> > * Pali Rohár <pali.rohar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> [150706 05:25]:
> > > into which file should I put documentation about new DT properties?
> > 
> > If it's Linux generic like linux,revision, then how about
> > Documentation/devicetree/bindings/revision.txt?
> > 
> > For the ATAGs, Documentation/devicetree/bindings/arm/atag.txt?
>
> Hm... now I'm thinking into which DT field should I put atags and 
> revision. In previous emails you wrote to use "linux,atags", now 
> "arm,atags"? And put them into root node? Or other?
> 
> In arch/arm/boot/compressed/atags_to_fdt.c code I see that most atag 
> properties are converted into "/chosen" node in DT...
> 
> So what do you prefer for "revision" and what for "atags"?

I'd prefer linux,revision and arm,atags. Chances are the ATAGs
won't be used on other architectures. I'm find with linux,atags
too if people prefer that.

Regards,

Tony
 
> Some mentioned examples:
> 
> "/revision"
> "/chosen/revision"
> "/linux,revision"
> "/chosen/linux,revision"
> ...
> 
> "/atags"
> "/chosen/atags"
> "/linux,atags"
> "/chosen/linux,atags"
> "/arm,atags"
> "/chosen/arm,atags"
> ...
> 
> -- 
> Pali Rohár
> pali.rohar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RESEND] [PATCH v2 1/2] arm: devtree: Set system_rev from DT revision
  2015-07-06 13:12                     ` Pali Rohár
  2015-07-06 13:55                       ` Tony Lindgren
@ 2015-07-06 15:22                       ` Rob Herring
       [not found]                         ` <CAL_Jsq+ysDqxTgfW1JvG5czHa4UkZQDv1BN63iK4jvV9kDXM8w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  1 sibling, 1 reply; 20+ messages in thread
From: Rob Herring @ 2015-07-06 15:22 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Tony Lindgren, Arnd Bergmann,
	linux-arm-kernel@lists.infradead.org, Russell King, Will Deacon,
	Ivaylo Dimitrov, Sebastian Reichel, Pavel Machek,
	Andreas Färber, linux-omap, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org

On Mon, Jul 6, 2015 at 8:12 AM, Pali Rohár <pali.rohar@gmail.com> wrote:
> On Monday 06 July 2015 14:31:27 Tony Lindgren wrote:
>> * Pali Rohár <pali.rohar@gmail.com> [150706 05:25]:
>> > into which file should I put documentation about new DT properties?
>>
>> If it's Linux generic like linux,revision, then how about
>> Documentation/devicetree/bindings/revision.txt?
>>
>> For the ATAGs, Documentation/devicetree/bindings/arm/atag.txt?
>>
>> Regards,
>>
>> Tony
>
> Hm... now I'm thinking into which DT field should I put atags and
> revision. In previous emails you wrote to use "linux,atags", now
> "arm,atags"? And put them into root node? Or other?
>
> In arch/arm/boot/compressed/atags_to_fdt.c code I see that most atag
> properties are converted into "/chosen" node in DT...
>
> So what do you prefer for "revision" and what for "atags"?
>
> Some mentioned examples:
>
> "/revision"

This one. This is a top level h/w property.

> "/chosen/revision"
> "/linux,revision"
> "/chosen/linux,revision"
> ...
>
> "/atags"
> "/chosen/atags"
> "/linux,atags"
> "/chosen/linux,atags"

This one. ATAGs are a Linux data struct.

Rob

> "/arm,atags"
> "/chosen/arm,atags"
> ...
>
> --
> Pali Rohár
> pali.rohar@gmail.com

^ permalink raw reply	[flat|nested] 20+ messages in thread

[parent not found: <CAL_Jsq+ysDqxTgfW1JvG5czHa4UkZQDv1BN63iK4jvV9kDXM8w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]

* Re: [RESEND] [PATCH v2 1/2] arm: devtree: Set system_rev from DT revision
       [not found]                         ` <CAL_Jsq+ysDqxTgfW1JvG5czHa4UkZQDv1BN63iK4jvV9kDXM8w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2015-07-06 16:20                           ` Pali Rohár
  2015-07-06 16:36                             ` Pali Rohár
  2015-07-06 17:30                             ` Rob Herring
  0 siblings, 2 replies; 20+ messages in thread
From: Pali Rohár @ 2015-07-06 16:20 UTC (permalink / raw)
  To: Rob Herring
  Cc: Tony Lindgren, Arnd Bergmann,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Russell King, Will Deacon, Ivaylo Dimitrov, Sebastian Reichel,
	Pavel Machek, Andreas Färber, linux-omap,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

[-- Attachment #1: Type: Text/Plain, Size: 1842 bytes --]

On Monday 06 July 2015 17:22:58 Rob Herring wrote:
> On Mon, Jul 6, 2015 at 8:12 AM, Pali Rohár <pali.rohar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > On Monday 06 July 2015 14:31:27 Tony Lindgren wrote:
> >> * Pali Rohár <pali.rohar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> [150706 05:25]:
> >> > into which file should I put documentation about new DT
> >> > properties?
> >> 
> >> If it's Linux generic like linux,revision, then how about
> >> Documentation/devicetree/bindings/revision.txt?
> >> 
> >> For the ATAGs, Documentation/devicetree/bindings/arm/atag.txt?
> >> 
> >> Regards,
> >> 
> >> Tony
> > 
> > Hm... now I'm thinking into which DT field should I put atags and
> > revision. In previous emails you wrote to use "linux,atags", now
> > "arm,atags"? And put them into root node? Or other?
> > 
> > In arch/arm/boot/compressed/atags_to_fdt.c code I see that most
> > atag properties are converted into "/chosen" node in DT...
> > 
> > So what do you prefer for "revision" and what for "atags"?
> > 
> > Some mentioned examples:
> > 
> > "/revision"
> 
> This one. This is a top level h/w property.
> 
> > "/chosen/revision"
> > "/linux,revision"
> > "/chosen/linux,revision"
> > ...
> > 
> > "/atags"
> > "/chosen/atags"
> > "/linux,atags"
> > "/chosen/linux,atags"
> 
> This one. ATAGs are a Linux data struct.
> 
> Rob
> 

Ok, and how read that property "/chosen/linux,atags" in function
setup_machine_fdt() from file arch/arm/kernel/devtree.c ?

of_get_flat_dt_prop() cannot be used unless somebody get me offset to
node "/chosen"...

Any idea?

> > "/arm,atags"
> > "/chosen/arm,atags"
> > ...
> > 
> > --
> > Pali Rohár
> > pali.rohar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org


-- 
Pali Rohár
pali.rohar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RESEND] [PATCH v2 1/2] arm: devtree: Set system_rev from DT revision
  2015-07-06 16:20                           ` Pali Rohár
@ 2015-07-06 16:36                             ` Pali Rohár
  2015-07-06 17:30                             ` Rob Herring
  1 sibling, 0 replies; 20+ messages in thread
From: Pali Rohár @ 2015-07-06 16:36 UTC (permalink / raw)
  To: Rob Herring
  Cc: Tony Lindgren, Arnd Bergmann,
	linux-arm-kernel@lists.infradead.org, Russell King, Will Deacon,
	Ivaylo Dimitrov, Sebastian Reichel, Pavel Machek,
	Andreas Färber, linux-omap, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org

[-- Attachment #1: Type: Text/Plain, Size: 845 bytes --]

On Monday 06 July 2015 18:20:35 Pali Rohár wrote:
> > > "/chosen/linux,atags"
> > 
> > This one. ATAGs are a Linux data struct.
> > 
> > Rob
> 
> Ok, and how read that property "/chosen/linux,atags" in function
> setup_machine_fdt() from file arch/arm/kernel/devtree.c ?
> 
> of_get_flat_dt_prop() cannot be used unless somebody get me offset to
> node "/chosen"...
> 
> Any idea?
> 

fdt_path_offset() from libfdt.h seems to work...

Is solution like this one acceptable?

#include <linux/libfdt.h>
#include "atags.h"

... setup_machine_fdt(unsigned int dt_phys) {

dt_virt = phys_to_virt(dt_phys);
dt_chosen = fdt_path_offset(dt_virt, "/chosen");
atags = of_get_flat_dt_prop(dt_chosen, "linux,atags", NULL);
save_atags(atags);

}

(this is without checks for errors)

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RESEND] [PATCH v2 1/2] arm: devtree: Set system_rev from DT revision
  2015-07-06 16:20                           ` Pali Rohár
  2015-07-06 16:36                             ` Pali Rohár
@ 2015-07-06 17:30                             ` Rob Herring
  1 sibling, 0 replies; 20+ messages in thread
From: Rob Herring @ 2015-07-06 17:30 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Tony Lindgren, Arnd Bergmann,
	linux-arm-kernel@lists.infradead.org, Russell King, Will Deacon,
	Ivaylo Dimitrov, Sebastian Reichel, Pavel Machek,
	Andreas Färber, linux-omap, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org

On Mon, Jul 6, 2015 at 11:20 AM, Pali Rohár <pali.rohar@gmail.com> wrote:
> On Monday 06 July 2015 17:22:58 Rob Herring wrote:
>> On Mon, Jul 6, 2015 at 8:12 AM, Pali Rohár <pali.rohar@gmail.com> wrote:
>> > On Monday 06 July 2015 14:31:27 Tony Lindgren wrote:
>> >> * Pali Rohár <pali.rohar@gmail.com> [150706 05:25]:
>> >> > into which file should I put documentation about new DT
>> >> > properties?
>> >>
>> >> If it's Linux generic like linux,revision, then how about
>> >> Documentation/devicetree/bindings/revision.txt?
>> >>
>> >> For the ATAGs, Documentation/devicetree/bindings/arm/atag.txt?
>> >>
>> >> Regards,
>> >>
>> >> Tony
>> >
>> > Hm... now I'm thinking into which DT field should I put atags and
>> > revision. In previous emails you wrote to use "linux,atags", now
>> > "arm,atags"? And put them into root node? Or other?
>> >
>> > In arch/arm/boot/compressed/atags_to_fdt.c code I see that most
>> > atag properties are converted into "/chosen" node in DT...
>> >
>> > So what do you prefer for "revision" and what for "atags"?
>> >
>> > Some mentioned examples:
>> >
>> > "/revision"
>>
>> This one. This is a top level h/w property.
>>
>> > "/chosen/revision"
>> > "/linux,revision"
>> > "/chosen/linux,revision"
>> > ...
>> >
>> > "/atags"
>> > "/chosen/atags"
>> > "/linux,atags"
>> > "/chosen/linux,atags"
>>
>> This one. ATAGs are a Linux data struct.
>>
>> Rob
>>
>
> Ok, and how read that property "/chosen/linux,atags" in function
> setup_machine_fdt() from file arch/arm/kernel/devtree.c ?
>
> of_get_flat_dt_prop() cannot be used unless somebody get me offset to
> node "/chosen"...

Why can't you get the offset yourself?

However, why does this need to be early? It is only used to populate
/proc, right?

Rob

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RESEND] [PATCH v2 1/2] arm: devtree: Set system_rev from DT revision
  2015-07-06 12:31                 ` Tony Lindgren
       [not found]                   ` <20150706123126.GG10705-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
@ 2015-07-06 15:20                   ` Rob Herring
  2015-07-06 15:24                     ` Tony Lindgren
  1 sibling, 1 reply; 20+ messages in thread
From: Rob Herring @ 2015-07-06 15:20 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Pali Rohár, Arnd Bergmann,
	linux-arm-kernel@lists.infradead.org, Russell King, Will Deacon,
	Ivaylo Dimitrov, Sebastian Reichel, Pavel Machek,
	Andreas Färber, linux-omap, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org

On Mon, Jul 6, 2015 at 7:31 AM, Tony Lindgren <tony@atomide.com> wrote:
> * Pali Rohár <pali.rohar@gmail.com> [150706 05:25]:
>>
>> into which file should I put documentation about new DT properties?
>
> If it's Linux generic like linux,revision, then how about

Just "revision" at the top level please. I'd prefer a string still,
but either is fine.

Rob

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RESEND] [PATCH v2 1/2] arm: devtree: Set system_rev from DT revision
  2015-07-06 15:20                   ` Rob Herring
@ 2015-07-06 15:24                     ` Tony Lindgren
  0 siblings, 0 replies; 20+ messages in thread
From: Tony Lindgren @ 2015-07-06 15:24 UTC (permalink / raw)
  To: Rob Herring
  Cc: Pali Rohár, Arnd Bergmann,
	linux-arm-kernel@lists.infradead.org, Russell King, Will Deacon,
	Ivaylo Dimitrov, Sebastian Reichel, Pavel Machek,
	Andreas Färber, linux-omap, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org

* Rob Herring <robherring2@gmail.com> [150706 08:23]:
> On Mon, Jul 6, 2015 at 7:31 AM, Tony Lindgren <tony@atomide.com> wrote:
> > * Pali Rohár <pali.rohar@gmail.com> [150706 05:25]:
> >>
> >> into which file should I put documentation about new DT properties?
> >
> > If it's Linux generic like linux,revision, then how about
> 
> Just "revision" at the top level please. I'd prefer a string still,
> but either is fine.

OK works for me thanks.

Tony

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RESEND] [PATCH v2 1/2] arm: devtree: Set system_rev from DT revision
  2015-05-06 11:44         ` Pali Rohár
  2015-06-25  5:01           ` Tony Lindgren
@ 2015-06-25 10:03           ` Russell King - ARM Linux
  1 sibling, 0 replies; 20+ messages in thread
From: Russell King - ARM Linux @ 2015-06-25 10:03 UTC (permalink / raw)
  To: Pali Rohár
  Cc: devicetree, Ivaylo Dimitrov, Arnd Bergmann, Tony Lindgren,
	Sebastian Reichel, Will Deacon, linux-kernel@vger.kernel.org,
	Pavel Machek, linux-omap, Andreas Färber, linux-arm-kernel

On Wed, May 06, 2015 at 01:44:17PM +0200, Pali Rohár wrote:
> On Wednesday 06 May 2015 13:04:01 Arnd Bergmann wrote:
> > What I mean is which code accesses this variable that early?
> > 
> 
> ATAG code is doing it at same early stage, so I added it to same early
> stage...

ATAG code does it early because ATAGs are only available early on, and
it's simpler to parse them all in one go, rather than having to do
multiple passes over the structure - especially when most instances are
just storing an integer to some BSS variable.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2015-07-06 17:30 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <1430902142-17035-1-git-send-email-pali.rohar@gmail.com>
     [not found] ` <1430902142-17035-2-git-send-email-pali.rohar@gmail.com>
2015-05-06  9:31   ` [RESEND] [PATCH v2 1/2] arm: devtree: Set system_rev from DT revision Arnd Bergmann
2015-05-06 10:37     ` Pali Rohár
2015-05-06 11:04       ` Arnd Bergmann
2015-05-06 11:44         ` Pali Rohár
2015-06-25  5:01           ` Tony Lindgren
2015-06-25  7:18             ` Pali Rohár
2015-06-25  7:22               ` Tony Lindgren
2015-06-25  7:27                 ` Pali Rohár
2015-06-25  7:41                   ` Tony Lindgren
     [not found]             ` <20150625050138.GL4156-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
2015-07-06 12:23               ` Pali Rohár
2015-07-06 12:31                 ` Tony Lindgren
     [not found]                   ` <20150706123126.GG10705-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
2015-07-06 13:12                     ` Pali Rohár
2015-07-06 13:55                       ` Tony Lindgren
2015-07-06 15:22                       ` Rob Herring
     [not found]                         ` <CAL_Jsq+ysDqxTgfW1JvG5czHa4UkZQDv1BN63iK4jvV9kDXM8w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-07-06 16:20                           ` Pali Rohár
2015-07-06 16:36                             ` Pali Rohár
2015-07-06 17:30                             ` Rob Herring
2015-07-06 15:20                   ` Rob Herring
2015-07-06 15:24                     ` Tony Lindgren
2015-06-25 10:03           ` Russell King - ARM Linux

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).