Re: [PATCH] Add support for binary includes.
David Gibson wrote: node { prop = /incbin/(path/to/data); }; node { prop = /incbin/(path/to/data, 8, 16); }; I still dislike the syntax, but haven't thought of a better one yet. There are some issues with the implementation too, but I've been a bit too busy with ePAPR stuff to review properly. I'm OK with the syntax, but whatever-ish. Would these be better?: prop = /call/(incbin, path/to/data, 17, 23); prop = /call[incbin]/(path/to/data); prop = /call incbin/(path/to/data, 12, 12+10); What is the aspect of the syntax that you don't like? I think we essentially need to stick in the /.../ realm to be consistent with the other non-standard names being used, like /include/. I can see a generalized form that allows other pre-defined or user-defined functions to be introduced and called or used in a similar way: prop = (22 + /fibonacci/(7)) 1000; prop = /directoryof/(/path/to/some/file.doc); interrupt-map = /pci_int_map/(8000, 2, 14); or whatever. We can paint this bikeshed for a long time if we need to. Or, we can get down to some serious issue if there are any. Are there? jdl ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] ps3: Fix unlikely incorrect usage
Fix unlikely(plug) == NO_IRQ into unlikely(plug == NO_IRQ). Signed-off-by: Samuel Tardieu [EMAIL PROTECTED] --- arch/powerpc/platforms/ps3/interrupt.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/powerpc/platforms/ps3/interrupt.c b/arch/powerpc/platforms/ps3/interrupt.c index 3a6db04..a14e5cd 100644 --- a/arch/powerpc/platforms/ps3/interrupt.c +++ b/arch/powerpc/platforms/ps3/interrupt.c @@ -709,7 +709,7 @@ static unsigned int ps3_get_irq(void) asm volatile(cntlzd %0,%1 : =r (plug) : r (x)); plug = 0x3f; - if (unlikely(plug) == NO_IRQ) { + if (unlikely(plug == NO_IRQ)) { pr_debug(%s:%d: no plug found: thread_id %lu\n, __func__, __LINE__, pd-thread_id); dump_bmp(per_cpu(ps3_private, 0)); -- 1.5.4.2.197.g22c43 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: How to dynamically disable/enable CPU features?
Original-Nachricht Datum: Sat, 23 Feb 2008 09:32:01 +1100 Von: Benjamin Herrenschmidt [EMAIL PROTECTED] An: Gerhard Pircher [EMAIL PROTECTED] CC: Milton Miller [EMAIL PROTECTED], linuxppc-dev@ozlabs.org Betreff: Re: How to dynamically disable/enable CPU features? The flag is in POSSIBLE. I now use this code in the platform probe function to nop out the code affected by the flag: cur_cpu_spec-cpu_features = ~CPU_FTR_NEED_COHERENT; /* Patch out unwanted feature. */ do_feature_fixups(cur_cpu_spec-cpu_features, PTRRELOC(__start___ftr_fixup), PTRRELOC(__stop___ftr_fixup)); It seems to work so far, but I would like to know if this is the right way to do it, or if calling do_feature_fixups() more than once can have any side effects. It's a bit hairy... Things -could- have been nop'ed out by the first call as a result of CPU_FTR_NEED_COHERENT being set and the second call will not be able to put them back in... now that may not be the case (depends what kind of patching is done with that flag) and so 'happen' to work for this specific bit but it isn't a nice solution... I checked this now. Looks like it only needs to nop out some code (mainly in the hash table code). A better long term approach is to look at moving the fixup to after the machine probe() after carefully checking whether that can cause any problem... Well, that's a job for an more experienced kernel developer. :) Thanks! Gerhard -- Pt! Schon vom neuen GMX MultiMessenger gehört? Der kann`s mit allen: http://www.gmx.net/de/go/multimessenger ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCHv4 2.6.25] i2c: adds support for i2c bus on Freescale CPM1/CPM2 controllers
Hi Olof, 2. record the I2c name in the dts tree, either as seperate tag (like linux,i2c-name=i2c-name) or as additional compatible entry (like compatible=..., linux,i2c-name). I have to say no on this one. The device tree is not supposed to know about how linux uses devices, there are firmwares out there that don't use DTS for thier device trees, etc. I still believe this this could be done for embedded devices which are usually booted via wrapper or U-Boot as those devices will most probably use the most exotic I2c devices out there (e.g. home-grown devices used by stbs). However, I'm not an device tree expert. 3. use a glue layer with a translation map. In my opinion this is an OK solution since the same information has to be added somewhere already anyway -- eiither to the drivers or to this translation table. It should of course be an abstacted shared table, preferrably contained under the i2c source directories since several platforms and architectures might share them. I could think of a mixture between 2. and 3.: Using the compatible attribute with the manufacturer stripped off as I2c name by default and using an exception table. For now, the struct i2c_driver_device would currently only need one entry (dallas,ds1374, rtc-ds1374). Thanks, Jochen ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] PowerPC 44x: add missing define TARGET_4xx to cuboot-taishan.c
On Thu, 21 Feb 2008 17:43:17 +0300 Valentine Barshak [EMAIL PROTECTED] wrote: In order to get the proper bd_info structure for PowerPC 440, both TARGET_4xx and TARGET_44x should be defined. Could you explain what this adds or why it's needed in the changelog? Also, is this needed for other 440 boards? josh ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCHv4 2.6.25] i2c: adds support for i2c bus on Freescale CPM1/CPM2 controllers
On 2/22/08, Jochen Friedrich [EMAIL PROTECTED] wrote: Hi Jean, +/* + * Wait for patch from Jon Smirl + * #include powerpc-common.h + */ It doesn't make sense to merge this comment upstream. I know you don't like the patch from Jon Smirl and you also explained your reasons. Fortunately, I2c no longer uses numeric device IDs but names. So what are the alternatives? 1. modify the I2c subsystem to accept OF names additionally to I2c names (proposed by Jon smirl). The correct statement is: modify the i2c subsystem to support the standard kernel driver aliasing mechanism. Leaving powerpc and OF out of the argument for the moment, i2c has a custom aliasing scheme on the x86 too. So as a first step, can we remove the custom i2c aliasing scheme and change i2c to use standard module aliases on the x86? Patches for this already exist. On 2/23/08, Jean Delvare [EMAIL PROTECTED] wrote: The problem I have with this is that it breaks compatibility. The chip name is not only used for device/driver matching, it is also exported to userspace as a sysfs attribute (name). Applications might rely on it. At least libsensors does. I think there is some confusion here. The OF aliases are only used by the kernel to load the correct driver. Would doing something like this help? static struct i2c_device_id pcf8563_id[] = { {pcf8563, 0, sysfs_legacy_name}, {rtc8564, 0, sysfs_legacy_name}, OF_ID(phillips,pcf8563, pcf8563_id[0], 0) OF_ID(epson,rtc8564, pcf8563_id[1], 0) {}, }; MODULE_DEVICE_TABLE(i2c, pcf8563_id); Then in the probe function you can use the pointer to find the base id entry and i2c never has to be aware that the OF alias exists. 2. record the I2c name in the dts tree, either as separate tag (like linux,i2c-name=i2c-name) Not really practical for the millions of machines (all PowerPC Macs) already shipped. or as additional compatible entry (like compatible=..., linux,i2c-name). 3. use a glue layer with a translation map. Audio codecs have exactly the same problem. There are probably other devices that also need mapping. This mapping table will need to contain a map from the OF names to the kernel driver names. It will need to stored permanently in RAM and contain all possible mappings. This table will only grow in size. The kernel has a widely used mechanism for mapping -- aliases in the device drivers. Why do we want to build a new, parallel one? What we are doing now is option 4. 4. Use kconfig to build custom kernels for each target system. Don't load drivers automatically based on what the BIOS tells us. -- Jon Smirl [EMAIL PROTECTED] ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCHv4 2.6.25] i2c: adds support for i2c bus on Freescale CPM1/CPM2 controllers
Hi, On Sun, Feb 24, 2008 at 04:16:30PM +0100, Jochen Friedrich wrote: Hi Olof, 2. record the I2c name in the dts tree, either as seperate tag (like linux,i2c-name=i2c-name) or as additional compatible entry (like compatible=..., linux,i2c-name). I have to say no on this one. The device tree is not supposed to know about how linux uses devices, there are firmwares out there that don't use DTS for thier device trees, etc. I still believe this this could be done for embedded devices which are usually booted via wrapper or U-Boot as those devices will most probably use the most exotic I2c devices out there (e.g. home-grown devices used by stbs). However, I'm not an device tree expert. Sorry, but you're wrong in your assumptions. Not all embedded devices use U-boot, and not all use the wrapper. It's just a bad idea to encode linux-specific things in the device tree, period. And even if you DO decide to go that route, guess what? You need a translation table just as with (3) anyway! 3. use a glue layer with a translation map. In my opinion this is an OK solution since the same information has to be added somewhere already anyway -- eiither to the drivers or to this translation table. It should of course be an abstacted shared table, preferrably contained under the i2c source directories since several platforms and architectures might share them. I could think of a mixture between 2. and 3.: Using the compatible attribute with the manufacturer stripped off as I2c name by default and using an exception table. For now, the struct i2c_driver_device would currently only need one entry (dallas,ds1374, rtc-ds1374). You still need the translation table, you're just flattening the namespace to one string instead of two, the same information still has to be encoded. I can't see what the benefit of this approach compared to the other one is. dallas,ds1374 already only has one translation entry in the table? -Olof ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] PowerPC 44x: add missing define TARGET_4xx to cuboot-taishan.c
On Sun, 24 Feb 2008 09:32:55 -0600 Josh Boyer [EMAIL PROTECTED] wrote: On Thu, 21 Feb 2008 17:43:17 +0300 Valentine Barshak [EMAIL PROTECTED] wrote: In order to get the proper bd_info structure for PowerPC 440, both TARGET_4xx and TARGET_44x should be defined. Could you explain what this adds or why it's needed in the changelog? Also, is this needed for other 440 boards? And actually, looking at this more, does taishan also need TARGET_440GX? josh ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] ps3: Fix unlikely incorrect usage
On 02/24/2008 12:06 AM, Samuel Tardieu wrote: Fix unlikely(plug) == NO_IRQ into unlikely(plug == NO_IRQ). Signed-off-by: Samuel Tardieu [EMAIL PROTECTED] --- arch/powerpc/platforms/ps3/interrupt.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) Thanks for the submission, but this fix is already in ps3-linux.git. I was waiting to collect a few other bugs before sending it to Paul. -Geoff ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 1/2][OF] Add of_device_is_disabled function
Josh Boyer wrote: On Sat, 23 Feb 2008 15:58:23 -0600 Josh Boyer [EMAIL PROTECTED] wrote: IEEE 1275 defined a standard status property to indicate the operational status of a device. The property has four possible values: okay, disabled, fail, fail-xxx. The absence of this property means the operational status of the device is unknown or okay. This adds a function called of_device_is_disabled that checks to see if a node has the status property set to disabled. This can be quite useful for devices that may be present but disabled due to pin sharing, etc. Talking with Ben H a bit, he suggested to reverse this API. Basically, create an of_device_is_available that returns 1 if the status property is completely missing, or if it's set to okay or ok. The latter is to cope with some broken firmwares. I agree with Ben's suggestion. The rtas_pci and eeh code could be converted to use this, which gives a net savings of a few bytes with ppc64_defconfig: diff --git a/arch/powerpc/kernel/rtas_pci.c b/arch/powerpc/kernel/rtas_pci.c index 433a0a0..39a752b 100644 --- a/arch/powerpc/kernel/rtas_pci.c +++ b/arch/powerpc/kernel/rtas_pci.c @@ -56,21 +56,6 @@ static inline int config_access_valid(struct pci_dn *dn, int where) return 0; } -static int of_device_available(struct device_node * dn) -{ -const char *status; - -status = of_get_property(dn, status, NULL); - -if (!status) -return 1; - -if (!strcmp(status, okay)) -return 1; - -return 0; -} - int rtas_read_config(struct pci_dn *pdn, int where, int size, u32 *val) { int returnval = -1; @@ -117,7 +102,7 @@ static int rtas_pci_read_config(struct pci_bus *bus, for (dn = busdn-child; dn; dn = dn-sibling) { struct pci_dn *pdn = PCI_DN(dn); if (pdn pdn-devfn == devfn -of_device_available(dn)) +of_device_is_available(dn)) return rtas_read_config(pdn, where, size, val); } @@ -164,7 +149,7 @@ static int rtas_pci_write_config(struct pci_bus *bus, for (dn = busdn-child; dn; dn = dn-sibling) { struct pci_dn *pdn = PCI_DN(dn); if (pdn pdn-devfn == devfn -of_device_available(dn)) +of_device_is_available(dn)) return rtas_write_config(pdn, where, size, val); } return PCIBIOS_DEVICE_NOT_FOUND; diff --git a/arch/powerpc/platforms/pseries/eeh.c b/arch/powerpc/platforms/pseries/eeh.c index 9eb539e..550b2f7 100644 --- a/arch/powerpc/platforms/pseries/eeh.c +++ b/arch/powerpc/platforms/pseries/eeh.c @@ -945,7 +945,6 @@ static void *early_enable_eeh(struct device_node *dn, void *data) unsigned int rets[3]; struct eeh_early_enable_info *info = data; int ret; - const char *status = of_get_property(dn, status, NULL); const u32 *class_code = of_get_property(dn, class-code, NULL); const u32 *vendor_id = of_get_property(dn, vendor-id, NULL); const u32 *device_id = of_get_property(dn, device-id, NULL); @@ -959,8 +958,8 @@ static void *early_enable_eeh(struct device_node *dn, void *data) pdn-eeh_freeze_count = 0; pdn-eeh_false_positives = 0; - if (status strncmp(status, ok, 2) != 0) - return NULL;/* ignore devices with bad status */ + if (!of_device_is_available(dn)) + return NULL; /* Ignore bad nodes. */ if (!class_code || !vendor_id || !device_id) ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
PIKA Warp Rev B
For those who like seeing pictures of embedded boards, here's a pic of rev B of the PIKA taco, official name Warp. As always, this is a development version. I only put one module on so you could see the 440EP in all it's glory. The dangley thing in the front is a combination button (the metal pad) and power leds. ftp://ftp.seanm.ca/stuff/taco-revb.jpg Also available at http:// same url. Cheers, Sean ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
RE: [PATCH] Xilinx: hwicap: cleanup
-Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Grant Likely Sent: Saturday, February 23, 2008 10:17 PM To: Stephen Neuendorffer Cc: linuxppc-dev@ozlabs.org; git-dev; [EMAIL PROTECTED]; [EMAIL PROTECTED] Subject: Re: [PATCH] Xilinx: hwicap: cleanup On Mon, Feb 11, 2008 at 11:24 AM, Stephen Neuendorffer [EMAIL PROTECTED] wrote: Fix some missing __user tags and incorrect section tags. Convert semaphores to mutexes. Make probed_devices re-entrancy and error condition safe. Fix some backwards memcpys. Some other minor cleanups. Use kerneldoc format. Signed-off-by: Stephen Neuendorffer [EMAIL PROTECTED] Thanks Steven, some more comments below. --- Grant, Since it appears that the driver will stay in as-is, here are the updates against mainline, based on Jiri's comments. --- drivers/char/xilinx_hwicap/buffer_icap.c | 80 ++-- drivers/char/xilinx_hwicap/fifo_icap.c | 60 +++--- drivers/char/xilinx_hwicap/xilinx_hwicap.c | 113 drivers/char/xilinx_hwicap/xilinx_hwicap.h | 24 +++--- 4 files changed, 148 insertions(+), 129 deletions(-) diff --git a/drivers/char/xilinx_hwicap/buffer_icap.c b/drivers/char/xilinx_hwicap/buffer_icap.c index dfea2bd..2c5d17d 100644 --- a/drivers/char/xilinx_hwicap/buffer_icap.c +++ b/drivers/char/xilinx_hwicap/buffer_icap.c @@ -148,9 +148,9 @@ static inline void buffer_icap_set_size(void __iomem *base_address, } /** - * buffer_icap_mSetoffsetReg: Set the bram offset register. - * @parameter base_address: contains the base address of the device. - * @parameter data: is the value to be written to the data register. + * buffer_icap_mSetoffsetReg - Set the bram offset register. This is the only function that is still in camel case; it should probably be changed also... In fact, this functions doesn't seem to be used at all. Can it just be removed? Are there any other unused functions in this driver? Actually, it's just the comment that still had the old name.. Fixed it. -Wall reports one unused static: drivers/char/xilinx_hwicap/xilinx_hwicap.c:240: warning: 'hwicap_command_capture' defined but not used I'd intended to leave this in, but I'm thinking it can be done by userspace code using this driver, so I took it out too. In verifying this, I discovered that I had inserted a variable names 'register', which doesn't work... Fixed that too. diff --git a/drivers/char/xilinx_hwicap/xilinx_hwicap.c b/drivers/char/xilinx_hwicap/xilinx_hwicap.c index 24f6aef..eddaa26 100644 --- a/drivers/char/xilinx_hwicap/xilinx_hwicap.c +++ b/drivers/char/xilinx_hwicap/xilinx_hwicap.c @@ -344,7 +345,7 @@ int hwicap_initialize_hwicap(struct hwicap_drvdata *drvdata) } static ssize_t -hwicap_read(struct file *file, char *buf, size_t count, loff_t *ppos) +hwicap_read(struct file *file, __user char *buf, size_t count, loff_t *ppos) This looks like it should be 'char __user *buf' instead of '__user char *buf'. Fixed. { struct hwicap_drvdata *drvdata = file-private_data; ssize_t bytes_to_read = 0; static ssize_t -hwicap_write(struct file *file, const char *buf, +hwicap_write(struct file *file, const __user char *buf, size_t count, loff_t *ppos) Ditto on placement of __user Fixed. @@ -549,8 +556,7 @@ static int hwicap_release(struct inode *inode, struct file *file) int i; int status = 0; - if (down_interruptible(drvdata-sem)) - return -ERESTARTSYS; + mutex_lock(drvdata-sem); Why not mutex_lock_interruptible()? (goes for all cases of mutex_lock()) It's not clear to me how to get 'correct' behavior in these functions if the interrupt happens. For instance in probe/setup, if the mutex_lock is interrupted, it doesn't appear that there is anything to do other than return an error code that no device is present? I think this was suggested by Jiri... Steve ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] [POWERPC] [v2] Xilinx: hwicap: cleanup
Fix some missing __user tags and incorrect section tags. Convert semaphores to mutexes. Make probed_devices re-entrancy and error condition safe. Fix some backwards memcpys. Some other minor cleanups. Use kerneldoc format. [v2] __user char = char __user removed unused hwicap_command_capture Fixed a comment that didn't match the function name fixed argument with 'register' keyword. Signed-off-by: Stephen Neuendorffer [EMAIL PROTECTED] --- Grant, Since it appears that the driver will stay in as-is, here are the updates against mainline, based on Jiri's comments. --- drivers/char/xilinx_hwicap/buffer_icap.c | 80 drivers/char/xilinx_hwicap/fifo_icap.c | 60 ++-- drivers/char/xilinx_hwicap/xilinx_hwicap.c | 138 +--- drivers/char/xilinx_hwicap/xilinx_hwicap.h | 24 +++--- 4 files changed, 144 insertions(+), 158 deletions(-) diff --git a/drivers/char/xilinx_hwicap/buffer_icap.c b/drivers/char/xilinx_hwicap/buffer_icap.c index dfea2bd..f577dae 100644 --- a/drivers/char/xilinx_hwicap/buffer_icap.c +++ b/drivers/char/xilinx_hwicap/buffer_icap.c @@ -73,8 +73,8 @@ #define XHI_BUFFER_START 0 /** - * buffer_icap_get_status: Get the contents of the status register. - * @parameter base_address: is the base address of the device + * buffer_icap_get_status - Get the contents of the status register. + * @base_address: is the base address of the device * * The status register contains the ICAP status and the done bit. * @@ -94,9 +94,9 @@ static inline u32 buffer_icap_get_status(void __iomem *base_address) } /** - * buffer_icap_get_bram: Reads data from the storage buffer bram. - * @parameter base_address: contains the base address of the component. - * @parameter offset: The word offset from which the data should be read. + * buffer_icap_get_bram - Reads data from the storage buffer bram. + * @base_address: contains the base address of the component. + * @offset: The word offset from which the data should be read. * * A bram is used as a configuration memory cache. One frame of data can * be stored in this storage buffer. @@ -108,8 +108,8 @@ static inline u32 buffer_icap_get_bram(void __iomem *base_address, } /** - * buffer_icap_busy: Return true if the icap device is busy - * @parameter base_address: is the base address of the device + * buffer_icap_busy - Return true if the icap device is busy + * @base_address: is the base address of the device * * The queries the low order bit of the status register, which * indicates whether the current configuration or readback operation @@ -121,8 +121,8 @@ static inline bool buffer_icap_busy(void __iomem *base_address) } /** - * buffer_icap_busy: Return true if the icap device is not busy - * @parameter base_address: is the base address of the device + * buffer_icap_busy - Return true if the icap device is not busy + * @base_address: is the base address of the device * * The queries the low order bit of the status register, which * indicates whether the current configuration or readback operation @@ -134,9 +134,9 @@ static inline bool buffer_icap_done(void __iomem *base_address) } /** - * buffer_icap_set_size: Set the size register. - * @parameter base_address: is the base address of the device - * @parameter data: The size in bytes. + * buffer_icap_set_size - Set the size register. + * @base_address: is the base address of the device + * @data: The size in bytes. * * The size register holds the number of 8 bit bytes to transfer between * bram and the icap (or icap to bram). @@ -148,9 +148,9 @@ static inline void buffer_icap_set_size(void __iomem *base_address, } /** - * buffer_icap_mSetoffsetReg: Set the bram offset register. - * @parameter base_address: contains the base address of the device. - * @parameter data: is the value to be written to the data register. + * buffer_icap_set_offset - Set the bram offset register. + * @base_address: contains the base address of the device. + * @data: is the value to be written to the data register. * * The bram offset register holds the starting bram address to transfer * data from during configuration or write data to during readback. @@ -162,9 +162,9 @@ static inline void buffer_icap_set_offset(void __iomem *base_address, } /** - * buffer_icap_set_rnc: Set the RNC (Readback not Configure) register. - * @parameter base_address: contains the base address of the device. - * @parameter data: is the value to be written to the data register. + * buffer_icap_set_rnc - Set the RNC (Readback not Configure) register. + * @base_address: contains the base address of the device. + * @data: is the value to be written to the data register. * * The RNC register determines the direction of the data transfer. It * controls whether a configuration or readback take place. Writing to @@ -178,10 +178,10 @@ static inline void buffer_icap_set_rnc(void __iomem *base_address, } /** - * buffer_icap_set_bram: Write
Use aliases instead of linux,network-index on Ebony
This patch alters the Ebony bootwrapper to use the new preferred method of using aliases to work out which MAC address to attach to which ethernet device node, rather than the old method based on the linux,network-index property. The now obsolete linux,network-index properties are removed from the ebony device tree as well. This won't break backwards compatiblity, because in cases where this fixup code is relevant, the device tree is part of the kernel image. Signed-off-by: David Gibson [EMAIL PROTECTED] For 2.6.26, tested on an Ebony board with treeboot. Index: working-2.6/arch/powerpc/boot/ebony.c === --- working-2.6.orig/arch/powerpc/boot/ebony.c 2008-02-25 11:22:31.0 +1100 +++ working-2.6/arch/powerpc/boot/ebony.c 2008-02-25 11:23:20.0 +1100 @@ -75,7 +75,8 @@ static void ebony_fixups(void) ibm440gp_fixup_clocks(sysclk, 6 * 1843200); ibm4xx_sdram_fixup_memsize(); - dt_fixup_mac_addresses(ebony_mac0, ebony_mac1); + dt_fixup_mac_address_by_alias(ethernet0, ebony_mac0); + dt_fixup_mac_address_by_alias(ethernet1, ebony_mac1); ibm4xx_fixup_ebc_ranges(/plb/opb/ebc); ebony_flashsel_fixup(); } Index: working-2.6/arch/powerpc/boot/dts/ebony.dts === --- working-2.6.orig/arch/powerpc/boot/dts/ebony.dts2008-02-25 11:25:28.0 +1100 +++ working-2.6/arch/powerpc/boot/dts/ebony.dts 2008-02-25 11:25:37.0 +1100 @@ -243,7 +243,6 @@ }; EMAC0: [EMAIL PROTECTED] { - linux,network-index = 0; device_type = network; compatible = ibm,emac-440gp, ibm,emac; interrupt-parent = UIC1; @@ -263,7 +262,6 @@ zmii-channel = 0; }; EMAC1: [EMAIL PROTECTED] { - linux,network-index = 1; device_type = network; compatible = ibm,emac-440gp, ibm,emac; interrupt-parent = UIC1; -- 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] Add support for binary includes.
On Fri, Feb 22, 2008 at 12:12:25PM -0600, Jon Loeliger wrote: David Gibson wrote: node { prop = /incbin/(path/to/data); }; node { prop = /incbin/(path/to/data, 8, 16); }; I still dislike the syntax, but haven't thought of a better one yet. There are some issues with the implementation too, but I've been a bit too busy with ePAPR stuff to review properly. I'm OK with the syntax, but whatever-ish. Would these be better?: prop = /call/(incbin, path/to/data, 17, 23); prop = /call[incbin]/(path/to/data); prop = /call incbin/(path/to/data, 12, 12+10); Ugh, I like those even less. What is the aspect of the syntax that you don't like? Well, when I first objected, I didn't really know why. But contemplating your discussion below has helped by crystallise why this syntax is making my ugly sense tingle. 1) I've no problem with using /word/ for reserved words. It's what we're already doing, and importantly it's lexically distinct in all contexts, even in proximity to the lexically troublesome node and property names. 2) I've no problem with something(arg, arg, arg) function syntax. Functions are handy, and this is consistent with our least suprise to C programmers design principle. What I don't like is the combination of the two. Using the /word/ form in (1) suggests that each /word/ is a lexically distinct symbol with functions in different contexts: consider /dts-v1/, /include/, /memreserve/ - they're all used only in their own distinct context. Use of /word/s in (2) would suggest that each /word/ is just an identifier for a different function, and should all be usable in a similar grammtical context - which won't be true of /memreserve/, /dts-v1/ and any other truly lexically distinct symbols we need to add. I think we essentially need to stick in the /.../ realm to be consistent with the other non-standard names being used, like /include/. So, with the thoughts about, I come to the conclusion that, yes, we should stick to /.../ for things which really act like reserved words, but binary includes aren't one of those things. Unlike source includes, binary includes only make sense inside a property *value* (or to put it another way, an a context suitable for an expression). Which means the possible lexical confusion with node/property names that motivated the /foo/ form in the first place disappears. So, I see two good options for the binary include. Once is to treat binary includes as a new special operator. That operator (as it's own, lexically distinct symbol) can be represented by a /word/, but in that case, using it shouldn't look like a general function call. Or, we can treat binary includes as a built-in function, see below. I can see a generalized form that allows other pre-defined or user-defined functions to be introduced and called or used in a similar way: prop = (22 + /fibonacci/(7)) 1000; prop = /directoryof/(/path/to/some/file.doc); interrupt-map = /pci_int_map/(8000, 2, 14); or whatever. We can paint this bikeshed for a long time if we need to. Or, we can get down to some serious issue if there are any. Are there? So, I like the notion of functions like this, but with identifiers that aren't /word/s. Re-invoking the least surprise to C programmers principle, in general I think the identifiers should be as C identifiers (i.e. [a-zA-Z_][a-zA-Z0-9_]*). With one caveat, it's not essential but it might be worthwhile to make built-in function identifiers obviously distinct from user-defined ones (if we add those in future). PS. I do see one potential gotcha with C-like function syntax. As we add expression support, the obvious way to handle constructs like: prop = 0x1 0x2, string; Is to treat ',' as a bytestring concatenation operator. There's potentialy ambiguity between that usage and it's use as a separator for function arguments. Of course the same issue exists in C with its comma operator, so it's not a showstopper. -- 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Use aliases instead of linux,network-index on Ebony
On Mon, 25 Feb 2008 11:33:39 +1100 David Gibson [EMAIL PROTECTED] wrote: This patch alters the Ebony bootwrapper to use the new preferred method of using aliases to work out which MAC address to attach to which ethernet device node, rather than the old method based on the linux,network-index property. I like it. But do we have this new preferred method documented somewhere? Doesn't seem to be in booting-without-of.txt. It probably should be added there, and reference to the linux,network-index property removed if this is really preferred now. Can you add something to this patch for that? josh ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Use aliases instead of linux,network-index on Ebony
On Sun, Feb 24, 2008 at 10:34:43PM -0600, Josh Boyer wrote: On Mon, 25 Feb 2008 11:33:39 +1100 David Gibson [EMAIL PROTECTED] wrote: This patch alters the Ebony bootwrapper to use the new preferred method of using aliases to work out which MAC address to attach to which ethernet device node, rather than the old method based on the linux,network-index property. I like it. But do we have this new preferred method documented somewhere? Doesn't seem to be in booting-without-of.txt. It probably should be added there, and reference to the linux,network-index property removed if this is really preferred now. Can you add something to this patch for that? Hrm. linux,network-index was mentioned in b-w-o.txt, but to be honest I don't think it ever belonged there. It describes a bootloader to kernel interface, whereas network-index was always a bootwrapper internal hack, applicable only when the device tree and the fixup code were built into the one image. aliases, on the other hand do warrent wider mention, and aren't mentioned at all in b-w-o.txt, but that's a matter of larger scope than just getting rid of the network-index hack. -- 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev