From: Scott Wood <hidden> Date: 2007-03-16 17:26:48
This patchset supersedes patches 2 and 14-17 of my previous (Mar. 13)
patchset. It depends on patches 11-13 of that patchset (in addition to
the patches that have been applied). Patch 18 from that set (Add
linux,stdout-path to the dts files) should probably wait until u-boot has
been updated to handle a pre-existing /chosen node.
-Scott
From: Scott Wood <hidden> Date: 2007-03-16 17:27:58
Previously, ft_create_node() ignored the parent parameter, and instead
created the new node as a child of the root node. The node is now
created as a child of "parent".
Signed-off-by: Scott Wood <redacted>
---
This patch is the same as before, but with a more verbose commit message,
as requested by David.
arch/powerpc/boot/flatdevtree.c | 17 ++++++++++++-----
arch/powerpc/boot/flatdevtree.h | 1 +
2 files changed, 13 insertions(+), 5 deletions(-)
From: Scott Wood <hidden> Date: 2007-03-16 17:27:59
Add get_parent, create_node, and find_node_by_prop_value to dt_ops.
Currently only implemented by flatdevtree_misc.
Also, add a _str convenience wrapper for setprop.
Signed-off-by: Scott Wood <redacted>
---
This version of the patch fixes the off-by-one bug in setprop_str, and
doesn't include the finddevice_rel hook (cuboot no longer needs it, and
as David pointed out, it'd be better to make that the only finddevice
hook, which means that of.c has to implement it first).
arch/powerpc/boot/flatdevtree_misc.c | 42 +++++++++++++++++++++------
arch/powerpc/boot/ops.h | 51 ++++++++++++++++++++++++++++++++++
2 files changed, 83 insertions(+), 10 deletions(-)
@@ -37,6 +38,12 @@ struct dt_ops {constintbuflen);int(*setprop)(constvoid*phandle,constchar*name,constvoid*buf,constintbuflen);+void*(*get_parent)(constvoid*phandle);+/* The node must not already exist. */+void*(*create_node)(constvoid*parent,constchar*name);+void*(*find_node_by_prop_value)(constvoid*prev,+constchar*propname,+constchar*propval,intproplen);unsignedlong(*finalize)(void);};externstructdt_opsdt_ops;
@@ -89,6 +96,50 @@ static inline int setprop(void *devp, const char *name, void *buf, int buflen)return(dt_ops.setprop)?dt_ops.setprop(devp,name,buf,buflen):-1;}+staticinlineintsetprop_str(void*devp,constchar*name,constchar*buf)+{+if(dt_ops.setprop)+returndt_ops.setprop(devp,name,buf,strlen(buf)+1);++return-1;+}++staticinlinevoid*get_parent(constchar*devp)+{+returndt_ops.get_parent?dt_ops.get_parent(devp):NULL;+}++staticinlinevoid*create_node(constvoid*parent,constchar*name)+{+returndt_ops.create_node?dt_ops.create_node(parent,name):NULL;+}+++staticinlinevoid*find_node_by_prop_value(constvoid*prev,+constchar*propname,+constchar*propval,intproplen)+{+if(dt_ops.find_node_by_prop_value)+returndt_ops.find_node_by_prop_value(prev,propname,+propval,proplen);++returnNULL;+}++staticinlinevoid*find_node_by_prop_value_str(constvoid*prev,+constchar*propname,+constchar*propval)+{+returnfind_node_by_prop_value(prev,propname,propval,+strlen(propval)+1);+}++staticinlinevoid*find_node_by_devtype(constvoid*prev,+constchar*type)+{+returnfind_node_by_prop_value_str(prev,"device_type",type);+}+staticinlinevoid*malloc(u32size){return(platform_ops.malloc)?platform_ops.malloc(size):NULL;
From: Scott Wood <hidden> Date: 2007-03-16 17:28:02
xlate_reg() uses the ranges properties of a node's parentage to find the
absolute physical address of the node's registers.
The ns16550 driver uses this when no virtual-reg property is found.
Signed-off-by: Scott Wood <redacted>
---
This is the same as the previous xlate_reg patch; it's included
in this patchset because it must be applied after the dt_ops
patch and before the dt_set_memory patch.
arch/powerpc/boot/Makefile | 2 +-
arch/powerpc/boot/devtree.c | 207 +++++++++++++++++++++++++++++++++++++++++++
arch/powerpc/boot/ns16550.c | 9 ++-
arch/powerpc/boot/ops.h | 1 +
4 files changed, 216 insertions(+), 3 deletions(-)
create mode 100644 arch/powerpc/boot/devtree.c
@@ -0,0 +1,207 @@+/*+*DeviceTreefunctionsthataresemantic,notstructural+*+*Author:ScottWood<scottwood@freescale.com>+*+*Copyright(c)2007FreescaleSemiconductor,Inc.+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodifyit+*underthetermsoftheGNUGeneralPublicLicenseversion2aspublished+*bytheFreeSoftwareFoundation.+*/++#include"ops.h"+#include"string.h"++#define MAX_ADDR_CELLS 4+#define MAX_RANGES 8++staticintget_reg_format(void*node,u32*naddr,u32*nsize)+{+intgot_addr=0,got_size=0;++while(node&&!(got_addr&&got_size)){+if(!got_addr&&getprop(node,"#address-cells",+naddr,4)==4)+got_addr=1;+if(!got_size&&getprop(node,"#size-cells",+nsize,4)==4)+got_size=1;++node=get_parent(node);+}++if(*naddr<=0||*naddr>MAX_ADDR_CELLS||+*nsize<=0||*nsize>*naddr)+return0;++returngot_addr&&got_size;+}++staticintsub_reg(u32*reg,u32*sub,u32naddr)+{+inti,borrow=0;++for(i=0;i<naddr;i++){+intprev_borrow=borrow;+borrow=reg[i]<sub[i]+prev_borrow;+reg[i]-=sub[i]+prev_borrow;+}++return!borrow;+}++staticintadd_reg(u32*reg,u32*add,u32naddr,u32naddr_add)+{+inti,carry=0;+intdiff=naddr-naddr_add;++if(diff<0){+for(i=0;i<-diff;i++)+if(*add++!=0)+return0;++diff=0;+}++for(i=diff;i<naddr;i++){+u64tmp=(u64)reg[i]+add[i-diff]+carry;+carry=tmp>>32;+reg[i]=(u32)tmp;+}++return!carry;+}++/* It is assumed that if the first byte of reg fits in a+*range,thenthewholeregblockfits.Itisalsoassumed+*that#size-cells<=#address-cells.+*/+staticintcompare_reg(u32*reg,u32*range,u32*rangesize,+u32naddr,u32nsize)+{+inti,sizeoff=naddr-nsize;++for(i=0;i<naddr;i++){+if(reg[i]<range[i])+return0;+if(reg[i]>range[i])+break;+}++for(i=sizeoff;i<naddr;i++){+u32end=range[i]+rangesize[i-sizeoff];++if(reg[i]<end)+break;+if(reg[i]>end)+return0;+if(i==naddr&®[i]==end)+return0;+}++return1;+}++staticintfind_range(u32*reg,u32*ranges,intnaddr,+intnsize,intnparentaddr,intbuflen)+{+intnrange=naddr+nparentaddr+nsize;+inti;++for(i=0;i+nrange<=buflen;i+=nrange){+if(compare_reg(reg,ranges+i,+ranges+i+naddr+nparentaddr,+naddr,nsize))+returni;+}++return-1;+}++/* Currently only generic buses without special encodings are supported.+*Inparticular,PCIisnotsupported.Also,onlythebeginningofthe+*regblockistracked;sizeisignoredexceptinranges.+*/+intxlate_reg(void*node,intres,unsignedlong*addr,+unsignedlong*size)+{+u32last_addr[MAX_ADDR_CELLS];+u32buf[MAX_ADDR_CELLS*MAX_RANGES*3];+void*parent;+u64ret_addr,ret_size;+u32naddr,nsize,prev_naddr,prev_nsize;+intbuflen,offset;++parent=get_parent(node);+if(!parent)+return0;++if(!get_reg_format(parent,&naddr,&nsize))+return0;++if(nsize>2)+return0;++buflen=getprop(node,"reg",buf,sizeof(buf))/4;+offset=(naddr+nsize)*res;++if(buflen<offset+naddr+nsize)+return0;++memcpy(last_addr,buf+offset,4*naddr);++ret_size=buf[offset+naddr];+if(nsize==2){+ret_size<<=32;+ret_size|=buf[offset+naddr+1];+}++while((node=get_parent(node))){+prev_naddr=naddr;+prev_nsize=nsize;++if(!get_reg_format(node,&naddr,&nsize))+return0;++buflen=getprop(node,"ranges",+buf,sizeof(buf));+if(buflen<0)+continue;+if(buflen>sizeof(buf))+return0;++offset=find_range(last_addr,buf,prev_naddr,+prev_nsize,naddr,buflen/4);++if(offset<0)+return0;++if(!sub_reg(last_addr,buf+offset,prev_naddr)||+!add_reg(buf+offset+prev_naddr,last_addr,+naddr,prev_naddr))+return0;++memcpy(last_addr,buf+offset+prev_naddr,4*naddr);+}++if(naddr>2)+return0;++ret_addr=last_addr[0];+if(naddr==2){+ret_addr<<=32;+ret_addr|=last_addr[1];+}++if(sizeof(void*)==4&&+(ret_addr>=0x100000000ULL||ret_size>0x100000000ULL||+ret_addr+ret_size>0x100000000ULL))+return0;++*addr=ret_addr;+if(size)+*size=ret_size;++return1;+}
From: Scott Wood <hidden> Date: 2007-03-16 17:28:04
The --no-gzip option can be passed to the wrapper so that the kernel
image is included uncompressed into the zImage. This is intended for
bootloaders where the zImage itself can be compressed, or where boot time
is considered more important than kernel image size.
Signed-off-by: Scott Wood <redacted>
---
arch/powerpc/boot/wrapper | 24 +++++++++++++++++-------
1 files changed, 17 insertions(+), 7 deletions(-)
From: Scott Wood <hidden> Date: 2007-03-16 17:28:49
1. The dtb should be generated in $tmpdir, rather than the current directory.
Normally, $tmpdir is ".", so it doesn't matter, but the wrapper should obey
the -W option if it should be passed.
2. DTC has a habit of complaining about errors which are not really
errors. Thus, the -f option is now passed to it, to force it to generate
output regardless.
Signed-off-by: Scott Wood <redacted>
---
arch/powerpc/boot/wrapper | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
From: Scott Wood <hidden> Date: 2007-03-16 17:28:51
To allow more robust association of each network device node with an
index (such as is used by the firmware or an EEPROM to indicate MAC
addresses), a network device's node may specify the index explicitly.
Signed-off-by: Scott Wood <redacted>
---
Documentation/powerpc/booting-without-of.txt | 13 +++++++++++++
1 files changed, 13 insertions(+), 0 deletions(-)
@@ -1165,6 +1165,13 @@ platforms are moved over to use the flattened-device-tree model. - phy-handle : The phandle for the PHY connected to this ethernet controller.+ Recommended properties:++ - linux,network-index : This is the intended "index" of this+ network device. This is used by the bootwrapper to interpret+ MAC addresses passed by the firmware when no information other+ than indices is available to associate an address with a device.+ Example: ethernet@24000 {
@@ -1533,6 +1540,12 @@ platforms are moved over to use the flattened-device-tree model. - mac-address : list of bytes representing the ethernet address. - phy-handle : The phandle for the PHY connected to this controller.+ Recommended properties:+ - linux,network-index : This is the intended "index" of this+ network device. This is used by the bootwrapper to interpret+ MAC addresses passed by the firmware when no information other+ than indices is available to associate an address with a device.+ Example: ucc@2000 { device_type = "network";
From: Scott Wood <hidden> Date: 2007-03-16 17:28:53
This adds a library function that platforms can call to fill in the
/memory node with the specified start and size. #address-cells and
#size-cells must be the same, but can be either 1 or 2.
Signed-off-by: Scott Wood <redacted>
---
arch/powerpc/boot/devtree.c | 25 +++++++++++++++++++++++++
arch/powerpc/boot/ops.h | 2 +-
2 files changed, 26 insertions(+), 1 deletions(-)
@@ -205,3 +205,28 @@ int xlate_reg(void *node, int res, unsigned long *addr,return1;}++voiddt_set_memory(u64start,u64size,intncells)+{+void*devp;+u32mem[4];++if(ncells<1||ncells>2)+return;++mem[ncells-1]=(u32)start;+mem[ncells*2-1]=(u32)size;++if(ncells==2){+mem[0]=start>>32;+mem[2]=size>>32;+}++devp=finddevice("/memory");+if(!devp){+devp=create_node(NULL,"memory");+setprop_str(devp,"device_type","memory");+}++setprop(devp,"reg",mem,ncells*8);+}
From: Scott Wood <hidden> Date: 2007-03-16 17:28:55
This adds a library function that platform files can call to
set clock-frequency, bus-frequency, and timebase-frequency in
each CPU node.
Signed-off-by: Scott Wood <redacted>
---
arch/powerpc/boot/devtree.c | 11 +++++++++++
arch/powerpc/boot/ops.h | 1 +
2 files changed, 12 insertions(+), 0 deletions(-)
From: Scott Wood <hidden> Date: 2007-03-16 17:28:58
This adds a library function that platform code can call to set the mac
addresses of network devices with a linux,network-index property,
according to a caller-provided table of mac address pointers.
Signed-off-by: Scott Wood <redacted>
---
arch/powerpc/boot/devtree.c | 28 ++++++++++++++++++++++++++++
arch/powerpc/boot/ops.h | 1 +
2 files changed, 29 insertions(+), 0 deletions(-)
@@ -241,3 +241,31 @@ void dt_set_cpu_clocks(u32 clock, u32 bus, u32 timebase)setprop(node,"timebase-frequency",&timebase,4);}}++/* mac_table points to a table of mac addresses, where the index+*intothetableforagivendevicecorrespondstothe+*linux,network-indexpropertyofitsdevicenode.Network+*devicenodeswithoutsuchapropertywillnotbeassigned+*addressesbythisfunction.+*/+voiddt_set_mac_addresses(u8**mac_table,intnum_addrs)+{+void*node=NULL;++while((node=find_node_by_devtype(node,"network"))){+u32index;+u8dummy[6];++if(getprop(node,"linux,network-index",+&index,sizeof(index))!=sizeof(index))+continue;++if(index>=num_addrs)+continue;++if(getprop(node,"mac-address",dummy,6)==6)+setprop(node,"mac-address",mac_table[index],6);++setprop(node,"local-mac-address",mac_table[index],6);+}+}
From: Scott Wood <hidden> Date: 2007-03-16 17:29:01
This allows platform code to call set_cmdline to initialize
/chosen/bootargs with a command line passed by the bootloader.
Signed-off-by: Scott Wood <redacted>
---
arch/powerpc/boot/main.c | 2 +-
arch/powerpc/boot/ops.h | 1 +
2 files changed, 2 insertions(+), 1 deletions(-)
@@ -83,6 +83,7 @@ int xlate_reg(void *node, int res, unsigned long *addr, unsigned long *size);voiddt_set_memory(u64start,u64len,intncells);voiddt_set_cpu_clocks(u32clock,u32bus,u32timebase);voiddt_set_mac_addresses(u8**mac_table,intnum_addrs);+voidset_cmdline(constchar*buf);staticinlinevoid*finddevice(constchar*name){
From: Scott Wood <hidden> Date: 2007-03-16 17:29:14
This file describes the bd_t struct, which is used by old versions of
U-boot to pass information to the kernel. Platform code that needs to
interoperate with such firmware can use this; it should not be used for
anything new.
Signed-off-by: Scott Wood <redacted>
---
arch/powerpc/boot/ppcboot.h | 108 +++++++++++++++++++++++++++++++++++++++++++
1 files changed, 108 insertions(+), 0 deletions(-)
create mode 100644 arch/powerpc/boot/ppcboot.h
@@ -0,0 +1,108 @@+/*+*ThisinterfaceisusedforcompatibilitywitholdU-boots*ONLY*.+*Pleasedonotimitateorextendthis.+*/++/*+*(C)Copyright2000,2001+*WolfgangDenk,DENXSoftwareEngineering,wd@denx.de.+*+*Thisprogramisfreesoftware;youcanredistributeitand/or+*modifyitunderthetermsoftheGNUGeneralPublicLicenseas+*publishedbytheFreeSoftwareFoundation;eitherversion2of+*theLicense,or(atyouroption)anylaterversion.+*+*Thisprogramisdistributedinthehopethatitwillbeuseful,+*butWITHOUTANYWARRANTY;withouteventheimpliedwarrantyof+*MERCHANTABILITYorFITNESSFORAPARTICULARPURPOSE.Seethe+*GNUGeneralPublicLicenseformoredetails.+*+*YoushouldhavereceivedacopyoftheGNUGeneralPublicLicense+*alongwiththisprogram;ifnot,writetotheFreeSoftware+*Foundation,Inc.,59TemplePlace,Suite330,Boston,+*MA02111-1307USA+*/++#ifndef __PPCBOOT_H__+#define __PPCBOOT_H__++/*+*BoardinformationpassedtokernelfromPPCBoot+*+*include/asm-ppc/ppcboot.h+*/++#include"types.h"++typedefstructbd_info{+unsignedlongbi_memstart;/* start of DRAM memory */+unsignedlongbi_memsize;/* size of DRAM memory in bytes */+unsignedlongbi_flashstart;/* start of FLASH memory */+unsignedlongbi_flashsize;/* size of FLASH memory */+unsignedlongbi_flashoffset;/* reserved area for startup monitor */+unsignedlongbi_sramstart;/* start of SRAM memory */+unsignedlongbi_sramsize;/* size of SRAM memory */+#if defined(TARGET_8xx) || defined(TARGET_CPM2) || defined(TARGET_85xx) ||\+defined(TARGET_83xx)+unsignedlongbi_immr_base;/* base of IMMR register */+#endif+#if defined(TARGET_PPC_MPC52xx)+unsignedlongbi_mbar_base;/* base of internal registers */+#endif+unsignedlongbi_bootflags;/* boot / reboot flag (for LynxOS) */+unsignedlongbi_ip_addr;/* IP Address */+unsignedcharbi_enetaddr[6];/* Ethernet address */+unsignedshortbi_ethspeed;/* Ethernet speed in Mbps */+unsignedlongbi_intfreq;/* Internal Freq, in MHz */+unsignedlongbi_busfreq;/* Bus Freq, in MHz */+#if defined(TARGET_CPM2)+unsignedlongbi_cpmfreq;/* CPM_CLK Freq, in MHz */+unsignedlongbi_brgfreq;/* BRG_CLK Freq, in MHz */+unsignedlongbi_sccfreq;/* SCC_CLK Freq, in MHz */+unsignedlongbi_vco;/* VCO Out from PLL, in MHz */+#endif+#if defined(TARGET_PPC_MPC52xx)+unsignedlongbi_ipbfreq;/* IPB Bus Freq, in MHz */+unsignedlongbi_pcifreq;/* PCI Bus Freq, in MHz */+#endif+unsignedlongbi_baudrate;/* Console Baudrate */+#if defined(TARGET_4xx)+unsignedcharbi_s_version[4];/* Version of this structure */+unsignedcharbi_r_version[32];/* Version of the ROM (IBM) */+unsignedintbi_procfreq;/* CPU (Internal) Freq, in Hz */+unsignedintbi_plb_busfreq;/* PLB Bus speed, in Hz */+unsignedintbi_pci_busfreq;/* PCI Bus speed, in Hz */+unsignedcharbi_pci_enetaddr[6];/* PCI Ethernet MAC address */+#endif+#if defined(TARGET_HYMOD)+hymod_conf_tbi_hymod_conf;/* hymod configuration information */+#endif+#if defined(TARGET_EVB64260) || defined(TARGET_405EP) || defined(TARGET_44x) || \+defined(TARGET_85xx)||defined(TARGET_83xx)+/* second onboard ethernet port */+unsignedcharbi_enet1addr[6];+#define HAVE_ENET1ADDR+#endif+#if defined(TARGET_EVB64260) || defined(TARGET_440GX) || defined(TARGET_85xx)+/* third onboard ethernet ports */+unsignedcharbi_enet2addr[6];+#define HAVE_ENET2ADDR+#endif+#if defined(TARGET_440GX)+/* fourth onboard ethernet ports */+unsignedcharbi_enet3addr[6];+#define HAVE_ENET3ADDR+#endif+#if defined(TARGET_4xx)+unsignedintbi_opbfreq;/* OB clock in Hz */+intbi_iic_fast[2];/* Use fast i2c mode */+#endif+#if defined(TARGET_440GX)+intbi_phynum[4];/* phy mapping */+intbi_phymode[4];/* phy mode */+#endif+}bd_t;++#define bi_tbfreq bi_intfreq++#endif /* __PPCBOOT_H__ */
From: Scott Wood <hidden> Date: 2007-03-16 17:29:14
Platforms beginning with "cuboot" have the bootwrapper code built in,
unlike the regular uboot platform.
Signed-off-by: Scott Wood <redacted>
---
arch/powerpc/boot/wrapper | 21 +++++++++++++++++++--
1 files changed, 19 insertions(+), 2 deletions(-)
From: Scott Wood <hidden> Date: 2007-03-16 17:29:16
This provides a function to call the wrapper script with a device tree
source file, as specified by CONFIG_DEVICE_TREE.
Signed-off-by: Scott Wood <redacted>
---
arch/powerpc/Kconfig | 16 ++++++++++++++++
arch/powerpc/boot/Makefile | 6 ++++++
2 files changed, 22 insertions(+), 0 deletions(-)
From: Scott Wood <hidden> Date: 2007-03-16 17:29:17
The cuImage target calls the wrapper with a cuboot platform chosen
based on the kernel config, as opposed to the uboot platform.
Signed-off-by: Scott Wood <redacted>
---
arch/powerpc/Makefile | 2 +-
arch/powerpc/boot/.gitignore | 3 +++
arch/powerpc/boot/Makefile | 10 ++++++++--
3 files changed, 12 insertions(+), 3 deletions(-)
@@ -173,7 +179,7 @@ image-$(CONFIG_PPC_CELLEB) += zImage.pseriesimage-$(CONFIG_PPC_CHRP)+=zImage.chrpimage-$(CONFIG_PPC_EFIKA)+=zImage.chrpimage-$(CONFIG_PPC_PMAC)+=zImage.pmac-image-$(CONFIG_DEFAULT_UIMAGE)+=uImage+image-$(CONFIG_DEFAULT_UIMAGE)+=uImagecuImage# For 32-bit powermacs, build the COFF and miboot images# as well as the ELF images.
From: Scott Wood <hidden> Date: 2007-03-16 17:29:19
This adds cuboot support for MPC83xx platforms.
A device tree used with this must have linux,stdout-path in /chosen and
linux,network-index in any network device nodes that need mac addresses
assigned.
Signed-off-by: Scott Wood <redacted>
---
arch/powerpc/boot/Makefile | 3 +-
arch/powerpc/boot/cuboot-83xx.c | 75 +++++++++++++++++++++++++++++++++++++++
2 files changed, 77 insertions(+), 1 deletions(-)
create mode 100644 arch/powerpc/boot/cuboot-83xx.c
From: David Gibson <hidden> Date: 2007-03-17 01:23:25
On Fri, Mar 16, 2007 at 12:27:52PM -0500, Scott Wood wrote:
Previously, ft_create_node() ignored the parent parameter, and instead
created the new node as a child of the root node. The node is now
created as a child of "parent".
Signed-off-by: Scott Wood <redacted>
Acked-by: David Gibson <redacted>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
From: David Gibson <hidden> Date: 2007-03-17 01:24:32
On Fri, Mar 16, 2007 at 12:27:54PM -0500, Scott Wood wrote:
Add get_parent, create_node, and find_node_by_prop_value to dt_ops.
Currently only implemented by flatdevtree_misc.
Also, add a _str convenience wrapper for setprop.
I think we may be able to simplify the hooks method structure a
little, but that can come later.
Acked-by: David Gibson <redacted>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
From: David Gibson <hidden> Date: 2007-03-17 01:25:30
On Fri, Mar 16, 2007 at 12:27:59PM -0500, Scott Wood wrote:
The --no-gzip option can be passed to the wrapper so that the kernel
image is included uncompressed into the zImage. This is intended for
bootloaders where the zImage itself can be compressed, or where boot time
is considered more important than kernel image size.
Why not.
Acked-by: David Gibson <redacted>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
From: David Gibson <hidden> Date: 2007-03-17 01:26:00
On Fri, Mar 16, 2007 at 12:28:46PM -0500, Scott Wood wrote:
To allow more robust association of each network device node with an
index (such as is used by the firmware or an EEPROM to indicate MAC
addresses), a network device's node may specify the index explicitly.
Signed-off-by: Scott Wood <redacted>
Acked-by: David Gibson david@gibson.dropbear.id.au>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
From: David Gibson <hidden> Date: 2007-03-17 01:26:54
On Fri, Mar 16, 2007 at 12:28:48PM -0500, Scott Wood wrote:
This adds a library function that platforms can call to fill in the
/memory node with the specified start and size. #address-cells and
#size-cells must be the same, but can be either 1 or 2.
Signed-off-by: Scott Wood <redacted>
Heh, you read my mind. I was in the process of making up a patch with
a helper function almost identical to this.
However, #address-cells=2, #size-cells=1 is common enough that we
really need to support that case.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
From: David Gibson <hidden> Date: 2007-03-17 01:27:15
On Fri, Mar 16, 2007 at 12:28:49PM -0500, Scott Wood wrote:
Signed-off-by: Scott Wood <redacted>
Acked-by: David Gibson <redacted>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
From: David Gibson <hidden> Date: 2007-03-17 01:28:24
On Fri, Mar 16, 2007 at 12:28:51PM -0500, Scott Wood wrote:
This adds a library function that platform files can call to
set clock-frequency, bus-frequency, and timebase-frequency in
each CPU node.
Signed-off-by: Scott Wood <redacted>
And my patch-in-progress had a function almost exactly like this one,
too. However, not all systems have a notion of "bus-frequency" in the
cpu node, so better make that one optional (I suggest skip the setprop
if bus==0).
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
From: David Gibson <hidden> Date: 2007-03-17 01:31:59
On Fri, Mar 16, 2007 at 12:28:53PM -0500, Scott Wood wrote:
This adds a library function that platform code can call to set the mac
addresses of network devices with a linux,network-index property,
according to a caller-provided table of mac address pointers.
Signed-off-by: Scott Wood <redacted>
And I had a function similar to this one, too. But I think my version
is better. Extracting it out of the patch I had in progress:
void __dt_fixup_mac_addresses(u32 startindex, ...)
{
va_list ap;
u32 index = startindex;
void *devp;
void *addr;
va_start(ap, startindex);
while ((addr = va_arg(ap, void *))) {
devp = find_node_by_prop_value(NULL,
linux,network-index",
&index, sizeof(index));
if (devp)
setprop(devp, "local-mac-address", addr, 6);
}
va_end(ap);
}
Then a wrapper in the header file to make it more convenient:
#define dt_fixup_mac_addresses(...) \
__dt_fixup_mac_addresses(0, __VA_ARGS__, NULL)
With this a cuboot platform can simply do:
dt_fixup_mac_addresses(&bd.enetaddr, &bd.enet1addr, &bd.enet2addr);
or similar with as many parameters as are appropriate.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
From: David Gibson <hidden> Date: 2007-03-17 01:36:10
On Fri, Mar 16, 2007 at 12:28:57PM -0500, Scott Wood wrote:
This allows platform code to call set_cmdline to initialize
/chosen/bootargs with a command line passed by the bootloader.
Signed-off-by: Scott Wood <redacted>
Ugh, can we hold off on this and the other cmdline related one for a
little. I had a different approach for cleaning up the cmdline
handling to make it easier for the platform to set the command line.
Um.. patch below. It won't work, as is, because it depends on my
unfinished device tree utils patch, but with luck the idea will be
clear enough.
Index: working-2.6/arch/powerpc/boot/main.c
===================================================================
@@ -211,31 +211,22 @@ static struct addr_range prep_initrd(str*editthecommandlinepassedtovmlinux(bysetting/chosen/bootargs).*Thebufferisputinit'sownsectionsothattoolsmaylocateiteasier.*/-staticcharbuiltin_cmdline[COMMAND_LINE_SIZE]+staticcharcmdline[COMMAND_LINE_SIZE]__attribute__((__section__("__builtin_cmdline")));-staticvoidget_cmdline(char*buf,intsize)+staticvoidprep_cmdline(void){-void*devp;-intlen=strlen(builtin_cmdline);--buf[0]='\0';--if(len>0){/* builtin_cmdline overrides dt's /chosen/bootargs */-len=min(len,size-1);-strncpy(buf,builtin_cmdline,len);-buf[len]='\0';-}-elseif((devp=finddevice("/chosen")))-getprop(devp,"bootargs",buf,size);-}--staticvoidset_cmdline(char*buf)-{-void*devp;--if((devp=finddevice("/chosen")))-setprop(devp,"bootargs",buf,strlen(buf)+1);+if(cmdline[0]=='\0')+dt_path_getprop("/chosen","bootargs",cmdline,+COMMAND_LINE_SIZE-1);++printf("\n\rLinux/PowerPC load: %s",cmdline);+/* If possible, edit the command line */+if(console_ops.edit_cmdline)+console_ops.edit_cmdline(cmdline,COMMAND_LINE_SIZE);+printf("\n\r");+/* Put the command line back into the devtree for the kernel */+dt_fixup_prop_str("/chosen","bootargs",cmdline);}structplatform_opsplatform_ops;
@@ -247,9 +238,15 @@ void start(void){structaddr_rangevmlinux,initrd;kernel_entry_tkentry;-charcmdline[COMMAND_LINE_SIZE];unsignedlongft_addr=0;+/* Do this first, because malloc() could clobber the loader's+*commandline.Onlyusetheloadercommandlineifa+*built-incommandlinewasn'tsetbyanexternaltool*/+if((loader_info.cmdline_len>0)&&(cmdline[0]=='\0'))+memmove(cmdline,loader_info.cmdline,+min(loader_info.cmdline_len,COMMAND_LINE_SIZE-1));+if(console_ops.open&&(console_ops.open()<0))exit();if(platform_ops.fixups)
@@ -260,18 +257,7 @@ void start(void)vmlinux=prep_kernel();initrd=prep_initrd(vmlinux,loader_info.initrd_addr,loader_info.initrd_size);--/* If cmdline came from zimage wrapper or if we can edit the one-*inthedt,printitoutandeditit,ifpossible.-*/-if((strlen(builtin_cmdline)>0)||console_ops.edit_cmdline){-get_cmdline(cmdline,COMMAND_LINE_SIZE);-printf("\n\rLinux/PowerPC load: %s",cmdline);-if(console_ops.edit_cmdline)-console_ops.edit_cmdline(cmdline,COMMAND_LINE_SIZE);-printf("\n\r");-set_cmdline(cmdline);-}+prep_cmdline();printf("Finalizing device tree...");if(dt_ops.finalize)
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
From: Timur Tabi <hidden> Date: 2007-03-18 00:22:36
David Gibson wrote:
while ((addr = va_arg(ap, void *))) {
devp = find_node_by_prop_value(NULL,
linux,network-index",
&index, sizeof(index));
if (devp)
setprop(devp, "local-mac-address", addr, 6);
The problem with this version is that it only updates local-mac-address,
and only if it already exists. Scott's version also updates
mac-address. Also, in the future, we hope to eliminate *all* MAC
address entries from the DTS files, and the boot loader and/or
bootwrapper will add one.
From: David Gibson <hidden> Date: 2007-03-18 11:56:56
On Sat, Mar 17, 2007 at 07:22:27PM -0500, Timur Tabi wrote:
David Gibson wrote:
quoted
while ((addr = va_arg(ap, void *))) {
devp = find_node_by_prop_value(NULL,
linux,network-index",
&index, sizeof(index));
if (devp)
setprop(devp, "local-mac-address", addr, 6);
The problem with this version is that it only updates local-mac-address,
and only if it already exists.
Uh.. no. Both my version and Scott's use setprop. I think at present
that won't add the property if it doesn't exist, but that's a bug
which one of Scott's other patches fixes. In any case, they have the
same behaviour in terms of whether the property needs to exist first.
Scott's version also updates
mac-address.
Well, first, details shmetails. It's the interface and approach I
thin is better. Second, I don't think the zImage should be setting
mac-address anyway. In OF that property is based on what the
interface has been used for during booting, which the zImage doesn't
know. The kernel doesn't need mac-address if local-mac-address is
present, so we should just leave it out.
Also, in the future, we hope to eliminate *all* MAC
address entries from the DTS files, and the boot loader and/or
bootwrapper will add one.
I don't think that's really a good idea. The bootloader certainly
should be able to add the property if it's not there, but it seems
silly to make the bootloader do memmove()s to insert a new property
when it's basically just as easy to set up the device tree to allow an
in-place edit.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
From: Timur Tabi <hidden> Date: 2007-03-19 15:09:43
David Gibson wrote:
quoted
The problem with this version is that it only updates local-mac-address,
and only if it already exists.
Uh.. no. Both my version and Scott's use setprop.
Sorry, I was reading the code wrong. I saw "if (devp)" and I thought devp was a pointer
to the local-mac-address node.
Second, I don't think the zImage should be setting
mac-address anyway.
Normally, that's true. The problem is that device drivers first check mac-address and
then local-mac-address (see of_get_mac_address()). If the DTS define mac-address as
something other than 00-00-00-00-00-00, the drivers are going to see mac-address that and
use it.
Obviously, the DTS files shouldn't have mac-address in them. But I haven't gotten around
to cleaning that up, because I'm still waiting for the U-Boot maintainers to apply my
pre-requisite patches.
> In OF that property is based on what the
interface has been used for during booting, which the zImage doesn't
know. The kernel doesn't need mac-address if local-mac-address is
present, so we should just leave it out.
Perhaps mac-address should be deleted instead of just ignored? I don't know if that
breaks kexec.
I don't think that's really a good idea. The bootloader certainly
should be able to add the property if it's not there, but it seems
silly to make the bootloader do memmove()s to insert a new property
when it's basically just as easy to set up the device tree to allow an
in-place edit.
I think it's better if the DTS only specifies properties that the bootloader can't. We
already need the ability to insert properties, so what's wrong with using that? I think
it doesn't make sense for the DTS to specify a MAC address.
--
Timur Tabi
Linux Kernel Developer @ Freescale
From: David Gibson <hidden> Date: 2007-03-20 03:50:05
On Fri, Mar 16, 2007 at 12:27:57PM -0500, Scott Wood wrote:
xlate_reg() uses the ranges properties of a node's parentage to find the
absolute physical address of the node's registers.
The ns16550 driver uses this when no virtual-reg property is found.
This is a pretty large chunk of code for use on just some platforms.
Remind me why we can't just insist on the presence of virtual-reg?
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
From: David Gibson <hidden> Date: 2007-03-20 03:59:57
On Mon, Mar 19, 2007 at 10:09:39AM -0500, Timur Tabi wrote:
David Gibson wrote:
quoted
quoted
The problem with this version is that it only updates local-mac-address,
and only if it already exists.
Uh.. no. Both my version and Scott's use setprop.
Sorry, I was reading the code wrong. I saw "if (devp)" and I
thought devp was a pointer to the local-mac-address node.
local-mac-address isn't a node, it's a property. We never deal in
pointers (or handles) to properties.
quoted
Second, I don't think the zImage should be setting
mac-address anyway.
Normally, that's true. The problem is that device drivers first
check mac-address and then local-mac-address (see
of_get_mac_address()). If the DTS define mac-address as something
other than 00-00-00-00-00-00, the drivers are going to see
mac-address that and use it.
Since mac-address is by definition a runtime property, it should
*never* exist in a static dts.
Obviously, the DTS files shouldn't have mac-address in them. But I
haven't gotten around to cleaning that up, because I'm still waiting
for the U-Boot maintainers to apply my pre-requisite patches.
I don't see why fixing the dts files relies on u-boot changes. u-boot
*can* know what's going on and might wish to set the mac-address
property, but that's its business. The dts files which are used for
inclusion into a zImage should never have this property.
> In OF that property is based on what the
quoted
interface has been used for during booting, which the zImage doesn't
know. The kernel doesn't need mac-address if local-mac-address is
present, so we should just leave it out.
Perhaps mac-address should be deleted instead of just ignored? I
don't know if that breaks kexec.
Hrm, maybe.
quoted
I don't think that's really a good idea. The bootloader certainly
should be able to add the property if it's not there, but it seems
silly to make the bootloader do memmove()s to insert a new property
when it's basically just as easy to set up the device tree to allow an
in-place edit.
I think it's better if the DTS only specifies properties that the
bootloader can't. We already need the ability to insert properties,
so what's wrong with using that? I think it doesn't make sense for
the DTS to specify a MAC address.
It's just that every insert could require copying most of the blob,
which is a bit sucky.
I was thinking of making an extension to dtc for these properties that
are expected to be replaced in-place by the bootloader: a special
token representing a value to be filled in later, so something like:
clock-frequency = < _ >;
or
local-mac-address = [??????];
The blob produced would just replace the blanks with either all 0s or
all 1s, so it doesn't actually do anything new, but it would provide a
strong visual clue in the source as to which properties are supposed
to be filled in later. Would that overcome your reluctance to include
bootloader-replaced properties in the dts?
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
From: Timur Tabi <hidden> Date: 2007-03-20 14:00:33
David Gibson wrote:
local-mac-address isn't a node, it's a property. We never deal in
pointers (or handles) to properties.
Sorry, I sometimes use those terms interchangeably.
Since mac-address is by definition a runtime property, it should
*never* exist in a static dts.
But it does today. I think we need to be considerate of existing broken
installations. Someone could be running an older U-Boot with an older
DTS, and if they boot a future kernel with it, then
quoted
Obviously, the DTS files shouldn't have mac-address in them. But I
haven't gotten around to cleaning that up, because I'm still waiting
for the U-Boot maintainers to apply my pre-requisite patches.
I don't see why fixing the dts files relies on u-boot changes.
Because today's U-Boot will, on some platforms, write to mac-address and
only mac-address. If you remove that property from the DTS without
fixing U-Boot first, then U-Boot will never be able to pass the MAC
address to the kernel.
u-boot
*can* know what's going on and might wish to set the mac-address
property, but that's its business. The dts files which are used for
inclusion into a zImage should never have this property.
Bugs need to be fixed in the right order. You can't fix the DTS files
until you fix all the software that uses them first.
quoted
Perhaps mac-address should be deleted instead of just ignored? I
don't know if that breaks kexec.
Hrm, maybe.
Would the cuImage bootwrapper run in a kexec'd kernel? I hope not. I
don't know anything about kexec.
It's just that every insert could require copying most of the blob,
which is a bit sucky.
What if U-Boot were smart enough to insert all of the required items in
one shot, and then filled them in?
I was thinking of making an extension to dtc for these properties that
are expected to be replaced in-place by the bootloader: a special
token representing a value to be filled in later, so something like:
clock-frequency = < _ >;
or
local-mac-address = [??????];
The blob produced would just replace the blanks with either all 0s or
all 1s, so it doesn't actually do anything new, but it would provide a
strong visual clue in the source as to which properties are supposed
to be filled in later. Would that overcome your reluctance to include
bootloader-replaced properties in the dts?
No, because it would require DTC to be constantly in sync with U-Boot,
and that sounds like a lose-lose proposition to me. It takes months for
any U-Boot patch that I write to be accepted and applied.
For instance, last month, I posted a fix for an old bug in U-Boot that
prevents initrd from working with any OF kernel. U-Boot has had this
bug ever since it added OF support. Despite repeated emails to WD
himself, I haven't even gotten an acknowledgment from him as to whether
he'll apply it, let alone when.
So as you can imagine, I am very wary about any design that requires
U-Boot to be updated in concert with any other software.
From: Scott Wood <hidden> Date: 2007-03-20 16:34:27
On Tue, Mar 20, 2007 at 02:50:05PM +1100, David Gibson wrote:
On Fri, Mar 16, 2007 at 12:27:57PM -0500, Scott Wood wrote:
quoted
xlate_reg() uses the ranges properties of a node's parentage to find the
absolute physical address of the node's registers.
The ns16550 driver uses this when no virtual-reg property is found.
This is a pretty large chunk of code for use on just some platforms.
It'd be useful on most flatdevicetree platforms that don't have
physical addresses larger than virtual addresses...
Remind me why we can't just insist on the presence of virtual-reg?
We can, if that's what the general consensus is... but I'd prefer not to.
For one thing, it impairs mobility of the SOC register block -- the
knowledge of where the SOC is mapped would be contained both in the SOC
node and in the serial nodes (and have to be updated both places). This
could be a bit of a pain if macros and/or overlays are implemented.
And I don't really understand complaining about the wastage for platforms
that don't need it in this specific case, when it happens in many others:
flatdevtree code with true OF, command line editing when it can be edited
just fine in the bootloader, simple_alloc with true OF, gzip code when
not compressing vmlinux, etc. Not to mention the duplication of device
tree manipulation code that could have been shared with the kernel if
this stuff weren't forced into the wrapper (why is OF allowed to have a
special prom_init, but nothing else?).
If we really want to optimize for size, the approach of including
everything except small bits of code for specific other platforms
probably isn't the best one. Does anyone actually do the
wrap-kernels-separate-from-building thing?
-Scott
From: Jon Loeliger <hidden> Date: 2007-03-20 18:17:34
On Mon, 2007-03-19 at 10:09, Timur Tabi wrote:
Obviously, the DTS files shouldn't have mac-address in them. But I haven't gotten around
to cleaning that up, because I'm still waiting for the U-Boot maintainers to apply my
pre-requisite patches.
Some U-Boot Maintainers are still waiting for respun
patches that apply cleanly. :-)
jdl
From: David Gibson <hidden> Date: 2007-03-21 01:51:04
On Fri, Mar 16, 2007 at 12:28:45PM -0500, Scott Wood wrote:
1. The dtb should be generated in $tmpdir, rather than the current directory.
Normally, $tmpdir is ".", so it doesn't matter, but the wrapper should obey
the -W option if it should be passed.
This is still wrong (or at least insufficient). If we're building
with a target directory different from the source directory we
shouldn't add anything to the source directory, but $tmpdir still
will.
2. DTC has a habit of complaining about errors which are not really
errors. Thus, the -f option is now passed to it, to force it to generate
output regardless.
I'd really prefer not to add -f by default. I think getting rid of
the bogus warnings from dtc is a better idea.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
From: David Gibson <hidden> Date: 2007-03-21 02:54:47
On Tue, Mar 20, 2007 at 09:00:29AM -0500, Timur Tabi wrote:
David Gibson wrote:
quoted
local-mac-address isn't a node, it's a property. We never deal in
pointers (or handles) to properties.
Sorry, I sometimes use those terms interchangeably.
quoted
Since mac-address is by definition a runtime property, it should
*never* exist in a static dts.
But it does today. I think we need to be considerate of existing broken
installations. Someone could be running an older U-Boot with an older
DTS, and if they boot a future kernel with it, then
quoted
quoted
Obviously, the DTS files shouldn't have mac-address in them. But I
haven't gotten around to cleaning that up, because I'm still waiting
for the U-Boot maintainers to apply my pre-requisite patches.
I don't see why fixing the dts files relies on u-boot changes.
Because today's U-Boot will, on some platforms, write to mac-address and
only mac-address. If you remove that property from the DTS without
fixing U-Boot first, then U-Boot will never be able to pass the MAC
address to the kernel.
Ah, and it can't add the property if it's not there at all. Ok, I
think I understand now. But.. does u-boot directly use the dts files
from the kernel tree, or does it have its own copies?
quoted
u-boot
*can* know what's going on and might wish to set the mac-address
property, but that's its business. The dts files which are used for
inclusion into a zImage should never have this property.
Bugs need to be fixed in the right order. You can't fix the DTS files
until you fix all the software that uses them first.
quoted
quoted
Perhaps mac-address should be deleted instead of just ignored? I
don't know if that breaks kexec.
Hrm, maybe.
Would the cuImage bootwrapper run in a kexec'd kernel? I hope not. I
don't know anything about kexec.
The maybe was more in relation to should we delete the mac-address
property. But that would have the same brokenness as removing
mac-address from the dts on old, broken u-boot's that set mac-address
instead of local-mac-address.
quoted
It's just that every insert could require copying most of the blob,
which is a bit sucky.
What if U-Boot were smart enough to insert all of the required items in
one shot, and then filled them in?
I don't see how that's reasonably possible when the items in question
are properties in different nodes.
quoted
I was thinking of making an extension to dtc for these properties that
are expected to be replaced in-place by the bootloader: a special
token representing a value to be filled in later, so something like:
clock-frequency = < _ >;
or
local-mac-address = [??????];
The blob produced would just replace the blanks with either all 0s or
all 1s, so it doesn't actually do anything new, but it would provide a
strong visual clue in the source as to which properties are supposed
to be filled in later. Would that overcome your reluctance to include
bootloader-replaced properties in the dts?
No, because it would require DTC to be constantly in sync with U-Boot,
and that sounds like a lose-lose proposition to me. It takes months for
any U-Boot patch that I write to be accepted and applied.
Uh.. how does it require synchronization with u-boot? This is really
just a form of internal documentation in the dts which shows which
properties are expected to be present, but overwrriten by the
bootloader.
For instance, last month, I posted a fix for an old bug in U-Boot that
prevents initrd from working with any OF kernel. U-Boot has had this
bug ever since it added OF support. Despite repeated emails to WD
himself, I haven't even gotten an acknowledgment from him as to whether
he'll apply it, let alone when.
So as you can imagine, I am very wary about any design that requires
U-Boot to be updated in concert with any other software.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
From: David Gibson <hidden> Date: 2007-03-21 03:12:14
On Tue, Mar 20, 2007 at 11:34:22AM -0500, Scott Wood wrote:
On Tue, Mar 20, 2007 at 02:50:05PM +1100, David Gibson wrote:
quoted
On Fri, Mar 16, 2007 at 12:27:57PM -0500, Scott Wood wrote:
quoted
xlate_reg() uses the ranges properties of a node's parentage to find the
absolute physical address of the node's registers.
The ns16550 driver uses this when no virtual-reg property is found.
This is a pretty large chunk of code for use on just some platforms.
It'd be useful on most flatdevicetree platforms that don't have
physical addresses larger than virtual addresses...
quoted
Remind me why we can't just insist on the presence of virtual-reg?
We can, if that's what the general consensus is... but I'd prefer not to.
For one thing, it impairs mobility of the SOC register block -- the
knowledge of where the SOC is mapped would be contained both in the SOC
node and in the serial nodes (and have to be updated both places). This
could be a bit of a pain if macros and/or overlays are implemented.
Hrm, yeah, okay, I'm convinced. I hope to get a patch out later today
which is a merger of yours and my ideas for devtree.c, excluding
xlate_reg(). Can you rebase on top of that?
And I don't really understand complaining about the wastage for platforms
that don't need it in this specific case, when it happens in many others:
flatdevtree code with true OF, command line editing when it can be edited
just fine in the bootloader, simple_alloc with true OF, gzip code when
not compressing vmlinux, etc.
Um.. flatdevtree and simple_alloc aren't included on OF (they're
built, but not linked in). Command line editing will be probably be
included in some redundant cases at present, because it's in the same
module as the rest of the serial stuff, but that would be
straightforward to fix. zlib is always included, it would be nice to
avoid that, but it's a bit less straightforward (but I think we can do
it with some dummy store-only versions of the gunzip_util.c functions,
plus some more weak symbol magic).
Not to mention the duplication of device
tree manipulation code that could have been shared with the kernel if
this stuff weren't forced into the wrapper (why is OF allowed to have a
special prom_init, but nothing else?).
Legacy. We'd like to get rid of OF's special casing eventually.
If we really want to optimize for size, the approach of including
everything except small bits of code for specific other platforms
probably isn't the best one. Does anyone actually do the
wrap-kernels-separate-from-building thing?
But we *don't* include everything - only the modules that are actually
needed from wrapper.a will be linked in.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
2. DTC has a habit of complaining about errors which are not really
errors. Thus, the -f option is now passed to it, to force it to
generate
output regardless.
I hope this is a temporary thing, and you're working on
fixing dtc, and then remove this hack?
Segher
To allow more robust association of each network device node with an
index (such as is used by the firmware or an EEPROM to indicate MAC
addresses), a network device's node may specify the index explicitly.
+ - linux,network-index : This is the intended "index" of this
+ network device. This is used by the bootwrapper to interpret
+ MAC addresses passed by the firmware when no information other
+ than indices is available to associate an address with a device.
What a nasty thing, and quite a misnomer too. Linux
_already_ knows how to bind ethN to which device, based
on user preferences; what you are really after is a way
to label the several network devices in your device tree
for use by firmware.
There already is a completely generic such mechanism; it's
called "aliases".
And yes I still believe the flattened dev tree should
implement the "/aliases" node and drop the other alias
mechanism, the OF-defined way is so much more flexible.
It even shows up in /proc/device-tree :-)
Segher
No way. The /chosen node should _always_ exist; you might
argue for the bootwrapper creating it if it isn't there,
but don't only do it when setting "bootargs", just always
do it (and do it early).
Segher
Well, first, details shmetails. It's the interface and approach I
thin is better. Second, I don't think the zImage should be setting
mac-address anyway. In OF that property is based on what the
interface has been used for during booting, which the zImage doesn't
know. The kernel doesn't need mac-address if local-mac-address is
present, so we should just leave it out.
Exactly. "mac-address" is mostly historical anyway, from the
time that people used RARP and such. Nowadays, almost any network
interface on any board has a statically assigned MAC address and
so should use "local-mac-address".
It would be a good idea to remove any mention of "mac-address"
from the kernel :-)
quoted
Also, in the future, we hope to eliminate *all* MAC
address entries from the DTS files, and the boot loader and/or
bootwrapper will add one.
There *are* use cases for setting it statically in the DTS
(bring up activities, ...) -- so please don't make it
impossible to do so.
But yes, no "shipping" DTS should have it, preferably.
Segher
Normally, that's true. The problem is that device drivers first check
mac-address and
then local-mac-address (see of_get_mac_address()). If the DTS define
mac-address as
something other than 00-00-00-00-00-00, the drivers are going to see
mac-address that and
use it.
So fix that :-) It's a layering violation anyway, a device
driver has no business peering at "mac-address", that's info
for a way higher layer ;-)
Obviously, the DTS files shouldn't have mac-address in them. But I
haven't gotten around
to cleaning that up, because I'm still waiting for the U-Boot
maintainers to apply my
pre-requisite patches.
Well there's that, sure. Please put comments in your source
code saying this _is_ a hack and for _what_ and that it should
be removed when your dependency is fixed, and all is fine.
Perhaps mac-address should be deleted instead of just ignored?
That would be incorrect behaviour if you get passed a device
tree where "mac-address" is correctly set (i.e., the device
has been used for booting, or something).
I think it's better if the DTS only specifies properties that the
bootloader can't. We
already need the ability to insert properties, so what's wrong with
using that?
Agreed. It was good before, when the tools weren't powerful
enough; but now it's hardly worth saving a few cycles.
I think
it doesn't make sense for the DTS to specify a MAC address.
In a few cases it might (during development, for example).
Or maybe a crazy firmware passed you a DTS instead of a DTB,
who knows ;-)
Segher
xlate_reg() uses the ranges properties of a node's parentage to find
the
absolute physical address of the node's registers.
The ns16550 driver uses this when no virtual-reg property is found.
This is a pretty large chunk of code for use on just some platforms.
Remind me why we can't just insist on the presence of virtual-reg?
Because "virtual-reg" is evil and should preferably never
be used? On the other hand, address translation for directly
memory mapped devices via "ranges" properties is a darn
solid, quite flexible, and proven mechanism.
I didn't look at the actual code but it should be really
small; I hope it doesn't copy the Linux code with its
millions of workarounds for broken device trees -- we
should insist on correct trees, or even just fix them
if they *are* broken ;-)
Segher
-----Original Message-----
From: linuxppc-dev-bounces+b08248=3Dfreescale.com@ozlabs.org=20
[mailto:linuxppc-dev-bounces+b08248=3Dfreescale.com@ozlabs.org]=20
On Behalf Of Segher Boessenkool
Sent: Wednesday, March 21, 2007 8:31 AM
To: Wood Scott-B07421
Cc: linuxppc-dev@ozlabs.org
Subject: Re: [PATCH 06/17] Document the linux,network-index property.
=20
quoted
To allow more robust association of each network device node with an
index (such as is used by the firmware or an EEPROM to indicate MAC
addresses), a network device's node may specify the index=20
explicitly.
=20
quoted
+ - linux,network-index : This is the intended "index" of this
+ network device. This is used by the bootwrapper to interpret
+ MAC addresses passed by the firmware when no information other
+ than indices is available to associate an address=20
with a device.
=20
What a nasty thing, and quite a misnomer too. Linux
_already_ knows how to bind ethN to which device, based
on user preferences; what you are really after is a way
to label the several network devices in your device tree
for use by firmware.
=20
There already is a completely generic such mechanism; it's
called "aliases".
=20
And yes I still believe the flattened dev tree should
implement the "/aliases" node and drop the other alias
mechanism, the OF-defined way is so much more flexible.
It even shows up in /proc/device-tree :-)
Segher, what is the the 'other alias' mechanism you are referring
to that should be dropped? Is it this proposed linux,network-index
property? or something else?
Stuart
From: Timur Tabi <hidden> Date: 2007-03-21 15:01:56
David Gibson wrote:
Ah, and it can't add the property if it's not there at all. Ok, I
think I understand now. But.. does u-boot directly use the dts files
from the kernel tree, or does it have its own copies?
I'm not sure what you mean by that. You compile the DTS into a DTB, and
then make the DTB available to U-Boot. That can mean either storing it
in flash, or tftp'ing it into memory. When you boot Linux via the
U-Boot "bootm" command, you give it the address of the DTB in memory.
U-Boot than looks for various things and updates them. It also creates
a couple new things, like a 'chosen' section.
If the DTB is in flash, it copies it to RAM. Then it updates the
in-memory copy. It's very hack-ish, though. I think it just overwrites
various nodes as it pleases, and then reconnects everything.
The maybe was more in relation to should we delete the mac-address
property. But that would have the same brokenness as removing
mac-address from the dts on old, broken u-boot's that set mac-address
instead of local-mac-address.
I was talking about the bootwrapper deleting the mac-address property,
*after* U-Boot has processed the DTB and handed it to the kernel.
quoted
What if U-Boot were smart enough to insert all of the required items in
one shot, and then filled them in?
I don't see how that's reasonably possible when the items in question
are properties in different nodes.
Ok.
Uh.. how does it require synchronization with u-boot?
This is really
just a form of internal documentation in the dts which shows which
properties are expected to be present, but overwrriten by the
bootloader.
Ok, I understand now, but I don't know what value it has. I don't see
the difference, from the DTS point-of-view, between
local-mac-address = [ 00 00 00 00 00 00 ]
and
local-mac-address = [ ? ? ? ? ? ? ];
In both cases, the property exists in the DTS. The whole point was to
remove it from the DTS entirely, and let U-Boot realize that it needs to
be added. I don't want DTC to need to know what's missing from a DTS.
From: Milton Miller <hidden> Date: 2007-03-21 15:03:18
You missed two .gz in the code, which would result in ineffective caching.
Found by inspection.
-if [ -z "$cacheit" -o ! -f "$vmz.gz" -o "$vmz.gz" -ot "$kernel" ]; then
+if [ -z "$cacheit" -o ! -f "$vmz$gzip" -o "$vmz$gzip" -ot "$kernel" ]; then
You also need to add these generated files to clean-files in the Makefile.
milton
From: Timur Tabi <hidden> Date: 2007-03-21 15:09:51
Segher Boessenkool wrote:
There *are* use cases for setting it statically in the DTS
(bring up activities, ...) -- so please don't make it
impossible to do so.
U-Boot has always updated the MAC address properties before booting
Linux. Therefore, you must be talking about a non-U-Boot scenerio. If
so, then I don't know what change I could make that would break this.
If the [local-]mac-address property is in the DTS, then DTC will compile
it and include it in the DTB. That won't change.
From: Timur Tabi <hidden> Date: 2007-03-21 15:15:19
Segher Boessenkool wrote:
quoted
Normally, that's true. The problem is that device drivers first check
mac-address and
then local-mac-address (see of_get_mac_address()). If the DTS define
mac-address as
something other than 00-00-00-00-00-00, the drivers are going to see
mac-address that and
use it.
So fix that :-)
I'm working on it.
> It's a layering violation anyway, a device
driver has no business peering at "mac-address", that's info
for a way higher layer ;-)
Eh, I don't think I agree with that. mac-address is supposed to be used
if it exists, because it represents the "most recent MAC address". If
it's not set, then the driver checks local-mac-address.
See of_get_mac_address(), which I wrote.
quoted
Obviously, the DTS files shouldn't have mac-address in them. But I
haven't gotten around
to cleaning that up, because I'm still waiting for the U-Boot
maintainers to apply my
pre-requisite patches.
Well there's that, sure. Please put comments in your source
code saying this _is_ a hack and for _what_ and that it should
be removed when your dependency is fixed, and all is fine.
I don't consider it a hack. The U-Boot code checks for mac-address, and
then updates it if it exists. Then it checks for local-mac-address and
does the same. I don't see this code changing ever, because we're
always going to need to support older device trees that don't have just
local-mac-address.
quoted
Perhaps mac-address should be deleted instead of just ignored?
That would be incorrect behaviour if you get passed a device
tree where "mac-address" is correctly set (i.e., the device
has been used for booting, or something).
From: Jerry Van Baren <hidden> Date: 2007-03-21 15:25:53
Timur Tabi wrote:
David Gibson wrote:
quoted
Ah, and it can't add the property if it's not there at all. Ok, I
think I understand now. But.. does u-boot directly use the dts files
from the kernel tree, or does it have its own copies?
I'm not sure what you mean by that. You compile the DTS into a DTB, and
then make the DTB available to U-Boot. That can mean either storing it
in flash, or tftp'ing it into memory. When you boot Linux via the
U-Boot "bootm" command, you give it the address of the DTB in memory.
U-Boot than looks for various things and updates them. It also creates
a couple new things, like a 'chosen' section.
If the DTB is in flash, it copies it to RAM. Then it updates the
in-memory copy. It's very hack-ish, though. I think it just overwrites
various nodes as it pleases, and then reconnects everything.
FWIIW, I'm getting close to having a usable implementation of a new
u-boot command "fdt" that is a lot cleaner than the hack-ish automagical
bootm copying and updating (and uses libfdt - thanks, David!). I just
sent an update this morning:
<http://article.gmane.org/gmane.comp.boot-loaders.u-boot/27187>
[snip]
Best regards,
gvb
From: Timur Tabi <hidden> Date: 2007-03-21 15:56:07
Jerry Van Baren wrote:
FWIIW, I'm getting close to having a usable implementation of a new
u-boot command "fdt" that is a lot cleaner than the hack-ish automagical
bootm copying and updating (and uses libfdt - thanks, David!). I just
sent an update this morning:
<http://article.gmane.org/gmane.comp.boot-loaders.u-boot/27187>
I saw that. Once it gets incorporated into WD's tree, I'll take a look at it and see what
we can do with it.
--
Timur Tabi
Linux Kernel Developer @ Freescale
From: Scott Wood <hidden> Date: 2007-03-21 16:01:20
On Wed, Mar 21, 2007 at 02:49:30PM +0100, Segher Boessenkool wrote:
I didn't look at the actual code but it should be really
small; I hope it doesn't copy the Linux code with its
millions of workarounds for broken device trees -- we
should insist on correct trees, or even just fix them
if they *are* broken ;-)
It's around 1700 bytes. Most of the complexity is for dealing with
different #address-cells and #size-cells -- if we could assume a max of 2
for each, then it could be simplified quite a bit by using u64 instead of
arrays. However, that wouldn't work with PCI, which has 3 address cells.
-Scott
Segher, what is the the 'other alias' mechanism you are referring
to that should be dropped? Is it this proposed linux,network-index
property? or something else?
Just the
pic0: pic@700 {
...
}
labeling thing -- it becomes redundant when the flat tree
stuff would support OF-style aliases, so it can be phased
out then.
For who doesn't know the aliases thing, it looks like this:
/ {
aliases {
pic0 = "/some/path/to/pic@700";
pic1 = "/some/path/to/pic@800";
enet = "/some/path/to/some/ethernet";
enet0 = "/some/path/to/some/ethernet";
enet1 = "/some/path/to/some/other/ethernet";
enet2 = "/some/path/to/yet/another/ethernet";
etc. Note you can have multiple aliases to the same
node, that comes in quite handy sometimes (like "disk"
is the default boot disk, ...)
Path name resolution looks at the aliases whenever
a path name doesn't start with a '/' character.
Segher
It's a layering violation anyway, a device
driver has no business peering at "mac-address", that's info
for a way higher layer ;-)
Eh, I don't think I agree with that. mac-address is supposed to be
used if it exists, because it represents the "most recent MAC
address".
That's policy and belongs in user land, not in the kernel,
and certainly not in a device driver.
I don't see this code changing ever, because we're always going to
need to support older device trees
No you don't. People who need to stick with an older dev
tree can stick with an older bootwrapper and kernel too.
Or if you *have* to support broken device trees, you should
fix them up in the bootwrapper so the kernel doesn't have to
be aware of them.
Segher
From: Timur Tabi <hidden> Date: 2007-03-21 19:10:19
Segher Boessenkool wrote:
quoted
Eh, I don't think I agree with that. mac-address is supposed to be
used if it exists, because it represents the "most recent MAC address".
That's policy and belongs in user land, not in the kernel,
and certainly not in a device driver.
The device driver needs to know what MAC address to program into the hardware when it
initializes it. Everything else the driver needs is also in the device tree. So you're
saying the driver should ignore the MAC address fields, and then when it needs a MAC
address, it should ......... ?
--
Timur Tabi
Linux Kernel Developer @ Freescale
I didn't look at the actual code but it should be really
small; I hope it doesn't copy the Linux code with its
millions of workarounds for broken device trees -- we
should insist on correct trees, or even just fix them
if they *are* broken ;-)
It's around 1700 bytes. Most of the complexity is for dealing with
different #address-cells and #size-cells -- if we could assume a max
of 2
for each, then it could be simplified quite a bit by using u64 instead
of
arrays. However, that wouldn't work with PCI, which has 3 address
cells.
It is quite safe to assume a max of 3 for #a and a
max of 2 for #s.
One way to simplify the code (when writing it in C)
is to always store any address as the maximum supported
number of 32-bit integers (padding it with zeroes perhaps).
No need to micro-optimise this stuff, just make the code
real generic and simple, that'll buy you more in the end.
Segher
Eh, I don't think I agree with that. mac-address is supposed to be
used if it exists, because it represents the "most recent MAC
address".
That's policy and belongs in user land, not in the kernel,
and certainly not in a device driver.
The device driver needs to know what MAC address to program into the
hardware when it initializes it.
And it should use "local-mac-address" for that when it exists.
Everything else the driver needs is also in the device tree. So
you're saying the driver should ignore the MAC address fields, and
then when it needs a MAC address, it should ......... ?
No, I'm saying that you shouldn't use "mac-address" for
something it isn't meant for.
You aren't supposed to have a "mac-address" property for
devices that weren't used during firmware execution / booting
_at all_.
Segher
From: Timur Tabi <hidden> Date: 2007-03-21 19:39:49
Segher Boessenkool wrote:
No, I'm saying that you shouldn't use "mac-address" for
something it isn't meant for.
If the property exists in the device tree, then it should be used, no? Whether or not it
exists is not for the driver to decide.
What I hope to do is remove all traces of mac-address from the device trees. If it's not
in the DTS, then U-Boot won't add it, bootwrapper won't add it, the kernel won't add it,
and therefore it won't exist when the driver loads, so the driver won't use it.
You aren't supposed to have a "mac-address" property for
devices that weren't used during firmware execution / booting
_at all_.
I can't tell if we agree or disagree. Perhaps you should look at the code and tell me
what needs to be changed?
--
Timur Tabi
Linux Kernel Developer @ Freescale
No, I'm saying that you shouldn't use "mac-address" for
something it isn't meant for.
If the property exists in the device tree, then it should be used, no?
Whether or not it exists is not for the driver to decide.
The property doesn't describe anything about the device;
it merely tells you something about what the firmware did
during booting. How or what you use it for later is a
policy decision -- you could for example reuse the address
instead of doing a new RARP sequence. Big deal nowadays.
What I hope to do is remove all traces of mac-address from the device
trees.
We share that goal :-)
If it's not in the DTS, then U-Boot won't add it, bootwrapper won't
add it, the kernel won't add it, and therefore it won't exist when the
driver loads, so the driver won't use it.
quoted
You aren't supposed to have a "mac-address" property for
devices that weren't used during firmware execution / booting
_at all_.
I can't tell if we agree or disagree. Perhaps you should look at the
code and tell me what needs to be changed?
Just everything everywhere that mentions "mac-address" should be
completely and utterly eradicated :-)
And sure I understand you have to change one component at a
time -- seems to me uboot is the first step to fix?
Segher
From: Timur Tabi <hidden> Date: 2007-03-21 20:03:19
Segher Boessenkool wrote:
quoted
If the property exists in the device tree, then it should be used, no?
Whether or not it exists is not for the driver to decide.
The property doesn't describe anything about the device;
it merely tells you something about what the firmware did
during booting.
Ah, I see your point. But how else can the bootloader tell the kernel what MAC address to
use? You need to make sure that both use the same address. I don't think the kernel
command line supports MAC addresses.
Just everything everywhere that mentions "mac-address" should be
completely and utterly eradicated :-)
Are you saying that Linux should not acknowledge the existence of the mac-address
property, even though it's part of the OF spec?
And sure I understand you have to change one component at a
time -- seems to me uboot is the first step to fix?
Depends on what you mean by a fix. Although I understand your point that the MAC address
doesn't really belong in the device tree, I don't see any better place for it. So for
now, I'm going on the assumption that mac-address and local-mac-address are valid
properties, so it's just a question on *how* they should be supported. In that context,
the kernel has been updated already, and some of U-Boot has also. Well, I'm ignoring some
of the more obscure IBM systems, because I don't know anything about them. All that's
left is 85xx, 86xx, and 5xxx, and then I can clean up the DTS files, and then as far as
I'm concerned, I'm done.
--
Timur Tabi
Linux Kernel Developer @ Freescale
From: Olof Johansson <hidden> Date: 2007-03-21 20:08:09
On Wed, Mar 21, 2007 at 03:03:07PM -0500, Timur Tabi wrote:
Segher Boessenkool wrote:
quoted
quoted
If the property exists in the device tree, then it should be used, no?
Whether or not it exists is not for the driver to decide.
The property doesn't describe anything about the device;
it merely tells you something about what the firmware did
during booting.
Ah, I see your point. But how else can the bootloader tell the kernel what MAC address to
use? You need to make sure that both use the same address. I don't think the kernel
command line supports MAC addresses.
quoted
Just everything everywhere that mentions "mac-address" should be
completely and utterly eradicated :-)
Are you saying that Linux should not acknowledge the existence of the mac-address
property, even though it's part of the OF spec?
Why? It's a property of the device -- the mac address it was assigned by
the vendor. It might not be a property of the ethernet controller chip,
but of the adapter it is.
Depends on what you mean by a fix. Although I understand your point that the MAC address
doesn't really belong in the device tree, I don't see any better place for it. So for
Right, there's no other place that makes sense. Leave it in there.
now, I'm going on the assumption that mac-address and local-mac-address are valid
properties, so it's just a question on *how* they should be supported. In that context,
the kernel has been updated already, and some of U-Boot has also. Well, I'm ignoring some
of the more obscure IBM systems, because I don't know anything about them. All that's
left is 85xx, 86xx, and 5xxx, and then I can clean up the DTS files, and then as far as
I'm concerned, I'm done.
We're using them in our driver/device tree as well.
-Olof
If the property exists in the device tree, then it should be used,=20=
quoted
quoted
no? Whether or not it exists is not for the driver to decide.
The property doesn't describe anything about the device;
it merely tells you something about what the firmware did
during booting.
Ah, I see your point. But how else can the bootloader tell the kernel=20=
what MAC address to use?
It should use "local-mac-address". Quoting:
=93local-mac-address=94 S
Standard property name to specify preassigned network address.
prop-encoded-array: Array of six bytes encoded with encode-bytes.
Specifies the 48-bit IEEE 802.3-style Media Access Control (MAC)
(as specified in ISO/IEC 8802-3 : 1993 [B3]) address assigned to
the device represented by the package, of device type =93network=94,
containing this property. The absence of this property indicates
that the device does not have a permanently assigned MAC address.
quoted
Just everything everywhere that mentions "mac-address" should be
completely and utterly eradicated :-)
Are you saying that Linux should not acknowledge the existence of the=20=
mac-address property, even though it's part of the OF spec?
Oh it can acknowledge its existence, it just has no business
using its contents.
quoted
And sure I understand you have to change one component at a
time -- seems to me uboot is the first step to fix?
Depends on what you mean by a fix. Although I understand your point=20=
that the MAC address doesn't really belong in the device tree, I don't=20=
see any better place for it. So for now, I'm going on the assumption=20=
that mac-address and local-mac-address are valid properties, so it's=20=
just a question on *how* they should be supported.
Just use "local-mac-address", ignore "mac-address" completely,
be happy, end of world hunger. Or something like that.
In that context, the kernel has been updated already, and some of=20
U-Boot has also. Well, I'm ignoring some of the more obscure IBM=20
systems, because I don't know anything about them. All that's left is=20=
85xx, 86xx, and 5xxx, and then I can clean up the DTS files, and then=20=
as far as I'm concerned, I'm done.
I think you're on the right track and we are talking past
each other somehow. We'll see :-)
Segher
Are you saying that Linux should not acknowledge the existence of the
mac-address
property, even though it's part of the OF spec?
Why? It's a property of the device -- the mac address it was assigned
by
the vendor.
No it's not. That info is in the "local-mac-address"
property, instead.
"mac-address" contains the address OF used for the device;
it should only be there _if_ the firmware used the device,
and it can only be different (if it is present at all) from
the "local-mac-address" if you are on ancient hardware.
Don't use "mac-address" in flat device trees. Use
"local-mac-address" instead.
Segher
Just everything everywhere that mentions "mac-address" should be
completely and utterly eradicated :-)
FYI -
On a recent build of IBM ofw, on a 8844 booted from the ethernet@4,1
find /proc/device-tree/ -path \*ethernet@4,1\*
primero ~# find /proc/device-tree/ -path \*ethernet@4,*
That's a real OF so yes it should have the "mac-address"
property if it initialised the device.
However, no consumer (except maybe a second-stage bootloader,
and even then it's dubious on modern systems) should use it;
"local-mac-address" is what you want. Really.
Segher
From: David Gibson <hidden> Date: 2007-03-21 23:42:26
On Wed, Mar 21, 2007 at 02:32:55PM +0100, Segher Boessenkool wrote:
quoted
However, #address-cells=2, #size-cells=1 is common enough that we
really need to support that case.
On the root node?!? Who would do such a strange thing?
Ebony, for one, it works nicely for a 32-bit system with >32-bit bus.
Apple G5s do it too.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
From: David Gibson <hidden> Date: 2007-03-22 00:06:20
On Wed, Mar 21, 2007 at 10:01:48AM -0500, Timur Tabi wrote:
David Gibson wrote:
quoted
Ah, and it can't add the property if it's not there at all. Ok, I
think I understand now. But.. does u-boot directly use the dts files
from the kernel tree, or does it have its own copies?
I'm not sure what you mean by that. You compile the DTS into a DTB, and
then make the DTB available to U-Boot. That can mean either storing it
in flash, or tftp'ing it into memory. When you boot Linux via the
U-Boot "bootm" command, you give it the address of the DTB in memory.
U-Boot than looks for various things and updates them. It also creates
a couple new things, like a 'chosen' section.
If the DTB is in flash, it copies it to RAM. Then it updates the
in-memory copy. It's very hack-ish, though. I think it just overwrites
various nodes as it pleases, and then reconnects everything.
I mean, does the u-boot source tree have its own copies of the dts
files which are built into a dtb during the u-boot build process? Or
do you take the dts from the kernel tree and make the dtb from that
when you build a dtb aware u-boot for a particular machine?
quoted
The maybe was more in relation to should we delete the mac-address
property. But that would have the same brokenness as removing
mac-address from the dts on old, broken u-boot's that set mac-address
instead of local-mac-address.
I was talking about the bootwrapper deleting the mac-address property,
*after* U-Boot has processed the DTB and handed it to the kernel.
Yes, but if have a version of u-boot that *only* sets mac-address and
not local-mac-address, doing so would clobber the only information
about the MAC address we have. In any case, I don't think is relevant
for discussion of this function, because its only designed for use
with *non* device tree aware firmware.
quoted
quoted
What if U-Boot were smart enough to insert all of the required items in
one shot, and then filled them in?
I don't see how that's reasonably possible when the items in question
are properties in different nodes.
Ok.
quoted
Uh.. how does it require synchronization with u-boot?
This is really
just a form of internal documentation in the dts which shows which
properties are expected to be present, but overwrriten by the
bootloader.
Ok, I understand now, but I don't know what value it has. I don't see
the difference, from the DTS point-of-view, between
local-mac-address = [ 00 00 00 00 00 00 ]
and
local-mac-address = [ ? ? ? ? ? ? ];
In terms of the generated dtb output there is no difference. Well,
probably. It would It's
purely syntactic sugar / internal documentation.
In both cases, the property exists in the DTS. The whole point was to
remove it from the DTS entirely,
Well, no. You wanted to get rid of the property from the dts, I
didn't. What I'm suggesting here is an idea to addresses at least one
possible objection to having the properties in the dts: the fact that
with actual values there it looks like the tree is complete and it
might not be obvious that a bootloader *must* tweak values to produce
a working tree.
I think it's useful to document in the dts that certain properties are
expected to be there, even if their actual values have to be
determined during boot. This syntax allows a dts to show to someone
reading it that a property is expected, and what its expected size is,
but that the value must be filled in later. It's for the benefit of
people reading the dts, not programs. It has the nice additional
property that it lets the bootloader avoid extra memmove()s and
possibly string table scans.
and let U-Boot realize that it needs to
be added. I don't want DTC to need to know what's missing from a DTS.
It doesn't need to know, but neither does it hurt to know.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
From: David Gibson <hidden> Date: 2007-03-22 00:29:11
On Wed, Mar 21, 2007 at 07:57:33PM +0100, Segher Boessenkool wrote:
quoted
Segher, what is the the 'other alias' mechanism you are referring
to that should be dropped? Is it this proposed linux,network-index
property? or something else?
Just the
pic0: pic@700 {
...
}
labeling thing -- it becomes redundant when the flat tree
stuff would support OF-style aliases, so it can be phased
out then.
dtc labels are *not* an alias mechanism: they're essentially a
compile-time rather than run-time concept and they can reference
properties as well as nodes (and in fact I'd like eventually to extend
them to allow labels inside property values).
For who doesn't know the aliases thing, it looks like this:
/ {
aliases {
pic0 = "/some/path/to/pic@700";
pic1 = "/some/path/to/pic@800";
enet = "/some/path/to/some/ethernet";
enet0 = "/some/path/to/some/ethernet";
enet1 = "/some/path/to/some/other/ethernet";
enet2 = "/some/path/to/yet/another/ethernet";
Putting the aliases in a separate node is far less usable for the
things labels are useful for than putting the label directly on the
node. Replacing labels with the OF alias mechanism is not sensible.
That said, auto-generating OF-style aliases from labels for the
benefit of run-time users might be worthwhile. And using /aliases
would work about as well as the "linux,network-index" trick.
etc. Note you can have multiple aliases to the same
node, that comes in quite handy sometimes (like "disk"
is the default boot disk, ...)
Path name resolution looks at the aliases whenever
a path name doesn't start with a '/' character.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
However, #address-cells=2, #size-cells=1 is common enough that we
really need to support that case.
On the root node?!? Who would do such a strange thing?
Ebony, for one, it works nicely for a 32-bit system with >32-bit bus.
It means you cannot have a 4GB-or-bigger region below your
root node. Dunno if Ebony ever needs one, for I/O mapping
perhaps?
Apple G5s do it too.
And they do a an awful workaround for their memory node
because of this.
Is their any reason why you couldn't use #a=#s=2 on 32-bit
systems? The root node is one contiguous hunk of address
space, so the biggest (theoretically) possible size is equal
to the biggest address (+1). It's not hard to come up with
an example where you need a size of 4GB or more.
Segher
Segher, what is the the 'other alias' mechanism you are referring
to that should be dropped? Is it this proposed linux,network-index
property? or something else?
Just the
pic0: pic@700 {
...
}
labeling thing -- it becomes redundant when the flat tree
stuff would support OF-style aliases, so it can be phased
out then.
dtc labels are *not* an alias mechanism: they're essentially a
compile-time rather than run-time concept
Sure. For flat device trees, you can evaluate the OF-style
aliases at compile time, too.
and they can reference
properties as well as nodes
I didn't know this though. If that's useful (I don't see how
right now), you want to keep labels I suppose.
Putting the aliases in a separate node is far less usable for the
things labels are useful for than putting the label directly on the
node. Replacing labels with the OF alias mechanism is not sensible.
Well I dunno, it's used quite often in "real" OF to create
cross-references between the nodes, and I always found it
very handy. There's no semantic difference (except with
aliases you can refer to nodes below the alias), and no big
syntactic difference either (well with labels you spread the
"short names" all over the tree, no relation between them
is immediately obvious).
That said, auto-generating OF-style aliases from labels for the
benefit of run-time users might be worthwhile.
Or the other way around. Or both!
And using /aliases
would work about as well as the "linux,network-index" trick.
From: Timur Tabi <hidden> Date: 2007-03-22 15:13:32
David Gibson wrote:
I mean, does the u-boot source tree have its own copies of the dts
files which are built into a dtb during the u-boot build process?
No.
Or
do you take the dts from the kernel tree and make the dtb from that
when you build a dtb aware u-boot for a particular machine?
You don't build the DTB with U-Boot. You build the DTB completely separately from U-Boot.
As far as U-Boot concerned, until you actually boot the kernel, the DTB is just another
binary blob.
Yes, but if have a version of u-boot that *only* sets mac-address and
not local-mac-address, doing so would clobber the only information
about the MAC address we have.
That's true.
In any case, I don't think is relevant
for discussion of this function, because its only designed for use
with *non* device tree aware firmware.
Ok.
In terms of the generated dtb output there is no difference. Well,
probably. It would It's
purely syntactic sugar / internal documentation.
Then I suggest we just leave the compiler as is, and just update the U-Boot documentation
to specify what it does with the various device tree properties.
quoted
In both cases, the property exists in the DTS. The whole point was to
remove it from the DTS entirely,
Well, no. You wanted to get rid of the property from the dts, I
didn't. What I'm suggesting here is an idea to addresses at least one
possible objection to having the properties in the dts: the fact that
with actual values there it looks like the tree is complete and it
might not be obvious that a bootloader *must* tweak values to produce
a working tree.
Hmmm... I can understand that. But I still think documenting it in U-Boot is the easier
solution.
I think it's useful to document in the dts that certain properties are
expected to be there, even if their actual values have to be
determined during boot.
The only problem with this is that the list of properties needed/used by U-Boot can change
from one version of U-Boot to another, and I'd hate to have to update all of the DTS files
every time this happens.
For instance,
It has the nice additional
property that it lets the bootloader avoid extra memmove()s and
possibly string table scans.
True, but I don't see why we should go through such an effort to avoid these things.
Besides, JVB is almost done getting libfdt into U-Boot, and that should make all this moot.
--
Timur Tabi
Linux Kernel Developer @ Freescale
From: Jon Loeliger <hidden> Date: 2007-03-22 15:15:16
On Wed, 2007-03-21 at 19:06, David Gibson wrote:
I mean, does the u-boot source tree have its own copies of the dts
files which are built into a dtb during the u-boot build process?
There are not DTS files in U-Boot anymore. They are all
currently in the arch/powerpc/boot/dts directory, or some
other private home directory. :-)
Or
do you take the dts from the kernel tree and make the dtb from that
yes.
when you build a dtb aware u-boot for a particular machine?
Do it whenever you want. But it has to be downloaded
to RAM or found in flash on the board by U-Boot by the
time you want to do the hand-off to Linux. That is,
there is no need to "combine" it with U-Boot to make
it "dtb aware". U-Boot is still built independently of
any DT[SB] file entirely.
quoted
Ok, I understand now, but I don't know what value it has. I don't see
the difference, from the DTS point-of-view, between
local-mac-address = [ 00 00 00 00 00 00 ]
and
local-mac-address = [ ? ? ? ? ? ? ];
In terms of the generated dtb output there is no difference. Well,
probably. It would It's
purely syntactic sugar / internal documentation.
Right. It is more like "make it clear to the DTS file
reader that these fields are intended to be filled in by
the bootloader".
Well, no. You wanted to get rid of the property from the dts, I
didn't. What I'm suggesting here is an idea to addresses at least one
possible objection to having the properties in the dts: the fact that
with actual values there it looks like the tree is complete and it
might not be obvious that a bootloader *must* tweak values to produce
a working tree.
(nit) But let's not forget that there are cases where we _do_ want
the DTS to be complete too.
I think it's useful to document in the dts that certain properties are
expected to be there, even if their actual values have to be
determined during boot. This syntax allows a dts to show to someone
reading it that a property is expected, and what its expected size is,
but that the value must be filled in later. It's for the benefit of
people reading the dts, not programs.
From: Scott Wood <hidden> Date: 2007-03-22 16:22:46
On Wed, Mar 21, 2007 at 12:51:04PM +1100, David Gibson wrote:
On Fri, Mar 16, 2007 at 12:28:45PM -0500, Scott Wood wrote:
quoted
1. The dtb should be generated in $tmpdir, rather than the current directory.
Normally, $tmpdir is ".", so it doesn't matter, but the wrapper should obey
the -W option if it should be passed.
This is still wrong (or at least insufficient). If we're building
with a target directory different from the source directory we
shouldn't add anything to the source directory, but $tmpdir still
will.
No, it won't. The current directory is the object directory. I tested
it.
The tmpdir thing wasn't to fix that (since it's not broken); it was just
something I'd noticed when I was looking over the script.
quoted
2. DTC has a habit of complaining about errors which are not really
errors. Thus, the -f option is now passed to it, to force it to generate
output regardless.
I'd really prefer not to add -f by default. I think getting rid of
the bogus warnings from dtc is a better idea.
I agree, and have posted a patch to DTC do this.
-Scott
From: David Gibson <hidden> Date: 2007-03-23 03:19:56
On Thu, Mar 22, 2007 at 12:11:45PM +0100, Segher Boessenkool wrote:
quoted
quoted
quoted
Segher, what is the the 'other alias' mechanism you are referring
to that should be dropped? Is it this proposed linux,network-index
property? or something else?
Just the
pic0: pic@700 {
...
}
labeling thing -- it becomes redundant when the flat tree
stuff would support OF-style aliases, so it can be phased
out then.
dtc labels are *not* an alias mechanism: they're essentially a
compile-time rather than run-time concept
Sure. For flat device trees, you can evaluate the OF-style
aliases at compile time, too.
quoted
and they can reference
properties as well as nodes
I didn't know this though. If that's useful (I don't see how
right now), you want to keep labels I suppose.
They're of no use at present if you compile direct to dtb. They're
potentially useful, however, if you compile to asm output, because the
labels are transcribed into symbols within the asm.
The idea is intended to be useful for systems where the bootloader has
to do some poking of the device tree, but doesn't need to change the
size of any properties. In that case the bootloader can do all the
necessary fixups on the tree without *any* understanding of the flat
tree structure. It simply links in the device tree structure, and
symbols within it reference all the necessary points for adjustment.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson