Re: [PATCH] leds: Add options to have GPIO LEDs start on or keep their state
On Wed, 13 May 2009, Wolfram Sang wrote:
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 376fe07..66e7d75 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -141,9 +141,14 @@ struct gpio_led {
const char *name;
const char *default_trigger;
unsignedgpio;
- u8 active_low : 1;
- u8 retain_state_suspended : 1;
+ unsignedactive_low : 1;
+ unsignedretain_state_suspended : 1;
+ unsigneddefault_state : 2;
+ /* default_state should be one of LEDS_GPIO_DEFSTATE_(ON|OFF|KEEP) */
Any specific reason for the change from u8 to unsigned? Could be
mentioned in the patch description maybe. And what Sean mentioned :)
I should have mentioned that in the description. It didn't make sense to
me to declare a bit field with u8. An eight bit type that is one bit
wide? The field width overrides the type width, but I think it's better to
just use unsigned and only specify a width once.
Other than that:
Acked-by: Wolfram Sang w.s...@pengutronix.de
--
Pengutronix e.K. | Wolfram Sang|
Industrial Linux Solutions | http://www.pengutronix.de/ |
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] leds: Add options to have GPIO LEDs start on or keep their state
There already is a default-on trigger but there are problems with it.
For one, it's a inefficient way to do it and requires led trigger support
to be compiled in.
But the real reason is that is produces a glitch on the LED. The GPIO is
allocate with the LED *off*, then *later* when the trigger runs it is
turned back on. If the LED was already on via the GPIO's reset default or
action of the firmware, this produces a glitch where the LED goes from on
to off to on. While normally this is fast enough that it wouldn't be
noticeable to a human observer, there are still serious problems.
One is that there may be something else on the GPIO line, like a hardware
alarm or watchdog, that is fast enough to notice the glitch.
Another is that the kernel may panic before the LED is turned back on, thus
hanging with the LED in the wrong state. This is not just speculation, but
actually happened to me with an embedded system that has an LED which
should turn off when the kernel finishes booting, which was left in the
incorrect state due to a bug in the OF LED binding code.
We also let GPIO LEDs get their initial value from whatever the current
state of the GPIO line is. On some systems the LEDs are put into some
state by the firmware or hardware before Linux boots, and it is desired to
have them keep this state which is otherwise unknown to Linux.
This requires that the underlying GPIO driver support reading the value of
output GPIOs. Some drivers support this and some do not.
The platform device binding gains a field in the platform data
default_state that controls this. There are three constants defined to
select from on, off, or keeping the current state. The OpenFirmware
binding uses a property named default-state that can be set to on,
off, or keep. The default if the property isn't present is off.
Signed-off-by: Trent Piepho xy...@speakeasy.org
Acked-by: Grant Likely grant.lik...@secretlab.ca
---
Documentation/powerpc/dts-bindings/gpio/led.txt | 17 -
drivers/leds/leds-gpio.c| 21 ++---
include/linux/leds.h|9 +++--
3 files changed, 41 insertions(+), 6 deletions(-)
diff --git a/Documentation/powerpc/dts-bindings/gpio/led.txt
b/Documentation/powerpc/dts-bindings/gpio/led.txt
index 4fe14de..064db92 100644
--- a/Documentation/powerpc/dts-bindings/gpio/led.txt
+++ b/Documentation/powerpc/dts-bindings/gpio/led.txt
@@ -16,10 +16,17 @@ LED sub-node properties:
string defining the trigger assigned to the LED. Current triggers are:
backlight - LED will act as a back-light, controlled by the framebuffer
system
-default-on - LED will turn on
+default-on - LED will turn on, but see default-state below
heartbeat - LED double flashes at a load average based rate
ide-disk - LED indicates disk activity
timer - LED flashes at a fixed, configurable rate
+- default-state: (optional) The initial state of the LED. Valid
+ values are on, off, and keep. If the LED is already on or off
+ and the default-state property is set the to same value, then no
+ glitch should be produced where the LED momentarily turns off (or
+ on). The keep setting will keep the LED at whatever its current
+ state is, without producing a glitch. The default is off if this
+ property is not present.
Examples:
@@ -30,14 +37,22 @@ leds {
gpios = mcu_pio 0 1; /* Active low */
linux,default-trigger = ide-disk;
};
+
+ fault {
+ gpios = mcu_pio 1 0;
+ /* Keep LED on if BIOS detected hardware fault */
+ default-state = keep;
+ };
};
run-control {
compatible = gpio-leds;
red {
gpios = mpc8572 6 0;
+ default-state = off;
};
green {
gpios = mpc8572 7 0;
+ default-state = on;
};
}
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index d210905..b5fd008 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -76,7 +76,7 @@ static int __devinit create_gpio_led(const struct gpio_led
*template,
struct gpio_led_data *led_dat, struct device *parent,
int (*blink_set)(unsigned, unsigned long *, unsigned long *))
{
- int ret;
+ int ret, state;
/* skip leds that aren't available */
if (!gpio_is_valid(template-gpio)) {
@@ -99,11 +99,15 @@ static int __devinit create_gpio_led(const struct gpio_led
*template,
led_dat-cdev.blink_set = gpio_blink_set;
}
led_dat-cdev.brightness_set = gpio_led_set;
- led_dat-cdev.brightness = LED_OFF;
+ if (template-default_state == LEDS_GPIO_DEFSTATE_KEEP)
+ state = !!gpio_get_value(led_dat-gpio) ^ led_dat-active_low;
+ else
+ state = (template-default_state == LEDS_GPIO_DEFSTATE_ON);
+ led_dat-cdev.brightness = state
Re: [PATCH] powerpc: Update Warp to use leds-gpio driver
On Mon, 6 Apr 2009, Sean MacLennan wrote: Now that leds-gpio is a proper OF platform driver, the Warp can use the leds-gpio driver rather than the old out-of-kernel driver. One side-effect is the leds-gpio driver always turns the leds off while the old driver left them alone. So we have to set them back to the correct settings. Originally, I had the OF bindings support this feature, see http://article.gmane.org/gmane.linux.kernel/749094 Maybe this would be a better way to do it? It avoids the glitch in the leds, is less code overall, and can be use by other devices that might want this same behavior. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [GIT PULL] LED updates for 2.6.29
On Sun, 11 Jan 2009, Richard Purdie wrote: On Sat, 2009-01-10 at 21:43 -0800, Trent Piepho wrote: On Sun, 11 Jan 2009, Richard Purdie wrote: On Sat, 2009-01-10 at 04:31 -0800, Trent Piepho wrote: The LED tree makes more sense for what's left I think. There was a openfirmware gpio patch, but that's already gone in. What's left only touches led files and the device tree binding docs. AFAIK, there were no objections to the patches left. Ok, these are now queued in the LED tree: http://git.o-hand.com/cgit.cgi/linux-rpurdie-leds/log/ I did merge the last three patches in one and make some changes to deal with some other outstanding issues. Let me know ASAP if there are any problems. Since the last patch looks like it's just my three patches folded into one, shouldn't I be listed as the author and the primary signed off by? I made changes other than just merging the three together (the syspend/resume change and the bitfield parts in leds.h) so putting you as signed off by/authorship would not have been correct and I credited you in the commit message instead. I wanted to get the missing patches queued ASAP so I went with the way that does fit in the rules as you'd not have been happy if a modified patch was attributed to you. I'll put you as author and a signoff if you confirm thats acceptable. It doesn't seem right to merge someone's patches together, make a very small change, and then no longer credit them as the author. Seems like it defeats the purpose of the SOB lines for tracing the train of custody too. If someone looks to see where the code came from, it will look like you wrote it. Maybe Freescale will say Intel stole our code? Without the SOB, what record is there in git that Freescale gave permission to put the code in the kernel? I also put some significant effort into writing informative commit messages, which have been lost. Along with Grant's acks for my patches. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [GIT PULL] LED updates for 2.6.29
On Fri, 9 Jan 2009, Richard Purdie wrote: On Fri, 2009-01-09 at 12:37 -0800, Trent Piepho wrote: On Fri, 9 Jan 2009, Richard Purdie wrote: for the LED tree updates for 2.6.29. This includes some new drivers, bugfixes and a core improvement resulting in nicer code. Any chance of these patches getting accepted? They've been going back and forth for months now. [v2,1/4] leds: Support OpenFirmware led bindings http://patchwork.ozlabs.org/patch/13581/ [v2,2/4] leds: Add option to have GPIO LEDs start on http://patchwork.ozlabs.org/patch/13580/ [v2,3/4] leds: Let GPIO LEDs keep their current state http://patchwork.ozlabs.org/patch/13583/ [v2,4/4] leds: Use tristate property in platform data http://patchwork.ozlabs.org/patch/13584/ I think there was some confusion as to who was going to take them. Are the PPC people happy with them? If so I'll merge through the LED tree. There are these and a couple of other patches around which have got lost in the system. If there is time which I'm hoping there might be, I'll try and get a second LED tree merge in. The LED tree makes more sense for what's left I think. There was a openfirmware gpio patch, but that's already gone in. What's left only touches led files and the device tree binding docs. AFAIK, there were no objections to the patches left. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [GIT PULL] LED updates for 2.6.29
On Sun, 11 Jan 2009, Richard Purdie wrote: On Sat, 2009-01-10 at 04:31 -0800, Trent Piepho wrote: The LED tree makes more sense for what's left I think. There was a openfirmware gpio patch, but that's already gone in. What's left only touches led files and the device tree binding docs. AFAIK, there were no objections to the patches left. Ok, these are now queued in the LED tree: http://git.o-hand.com/cgit.cgi/linux-rpurdie-leds/log/ I did merge the last three patches in one and make some changes to deal with some other outstanding issues. Let me know ASAP if there are any problems. Since the last patch looks like it's just my three patches folded into one, shouldn't I be listed as the author and the primary signed off by? ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Status of fsl booke TLB and ATMU patches
2.6.28 is out, can my existing patches be accepted? [2/2] POWERPC/fsl-pci: Set relaxed ordering on prefetchable ranges http://patchwork.ozlabs.org/patch/14546/ [1/2] POWERPC/fsl-pci: Better ATMU setup http://patchwork.ozlabs.org/patch/14544/ [5/5] powerpc: booke: Allow larger CAM sizes than 256 MB http://patchwork.ozlabs.org/patch/12885/ [4/5] powerpc: booke: Make CAM entries used for lowmem configurable http://patchwork.ozlabs.org/patch/12884/ [3/5] powerpc: booke: Remove code duplication in lowmem mapping http://patchwork.ozlabs.org/patch/12883/ [2/5] powerpc: booke: Remove num_tlbcam_entries http://patchwork.ozlabs.org/patch/12882/ [1/5] powerpc: booke: Don't hard-code size of struct tlbcam http://patchwork.ozlabs.org/patch/12881/ ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH 1/2] POWERPC/fsl-pci: Better ATMU setup
The code that sets up the outbound ATMU windows, which is used to map CPU
physical addresses into PCI bus addresses where BARs will be mapped, didn't
work so well.
For one, it leaked the ioremap() of the ATMU registers. Another small bug
was the high 20 bits of the PCI bus address were left as zero. It's legal
for prefetchable memory regions to be above 32 bits, so the high 20 bits
might not be zero.
Mainly, it couldn't handle ranges that were not a power of two in size or
were not naturally aligned. The ATMU windows have these requirements (size
alignment), but the code didn't bother to check if the ranges it was
programming met them. If they didn't, the windows would silently be
programmed incorrectly.
This new code can handle ranges which are not power of two sized nor
naturally aligned. It simply splits the ranges into multiple valid ATMU
windows. As there are only four windows, pooly aligned or sized ranges
(which didn't even work before) may run out of windows. In this case an
error is printed and an effort is made to disable the unmapped resources.
An improvement that could be made would be to make use of the default
outbound window. Iff hose-pci_mem_offset is zero, then it's possible that
some or all of the ranges might not need an outbound window and could just
use the default window.
The default ATMU window can support a pci_mem_offset less than zero too,
but pci_mem_offset is unsigned. One could say the abilities allowed a
powerpc pci_controller is neither subset nor a superset of the abilities of
a Freescale PCIe controller. Thankfully, the most useful bits are in the
intersection of the two abilities.
Signed-off-by: Trent Piepho tpie...@freescale.com
---
arch/powerpc/sysdev/fsl_pci.c | 104 -
1 files changed, 71 insertions(+), 33 deletions(-)
diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index 5b264eb..656f914 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -28,62 +28,100 @@
#include sysdev/fsl_pci.h
#if defined(CONFIG_PPC_85xx) || defined(CONFIG_PPC_86xx)
+static int __init setup_one_atmu(struct ccsr_pci __iomem *pci,
+ unsigned int index, const struct resource *res,
+ resource_size_t offset)
+{
+ resource_size_t pci_addr = res-start - offset;
+ resource_size_t phys_addr = res-start;
+ resource_size_t size = res-end - res-start + 1;
+ u32 flags = 0x80044000; /* enable mem R/W */
+ unsigned int i;
+
+ pr_debug(PCI MEM resource start 0x%016llx, size 0x%016llx.\n,
+ (u64)res-start, (u64)size);
+
+ for (i = 0; size 0; i++) {
+ unsigned int bits = min(__ilog2(size),
+ __ffs(pci_addr | phys_addr));
+
+ if (index + i = 5)
+ return -1;
+
+ out_be32(pci-pow[index + i].potar, pci_addr 12);
+ out_be32(pci-pow[index + i].potear, (u64)pci_addr 44);
+ out_be32(pci-pow[index + i].powbar, phys_addr 12);
+ out_be32(pci-pow[index + i].powar, flags | (bits - 1));
+
+ pci_addr += (resource_size_t)1U bits;
+ phys_addr += (resource_size_t)1U bits;
+ size -= (resource_size_t)1U bits;
+ }
+
+ return i;
+}
+
/* atmu setup for fsl pci/pcie controller */
void __init setup_pci_atmu(struct pci_controller *hose, struct resource *rsrc)
{
struct ccsr_pci __iomem *pci;
- int i;
+ int i, j, n;
pr_debug(PCI memory map start 0x%016llx, size 0x%016llx\n,
(u64)rsrc-start, (u64)rsrc-end - (u64)rsrc-start + 1);
pci = ioremap(rsrc-start, rsrc-end - rsrc-start + 1);
+ if (!pci) {
+ dev_err(hose-parent, Unable to map ATMU registers\n);
+ return;
+ }
- /* Disable all windows (except powar0 since its ignored) */
+ /* Disable all windows (except powar0 since it's ignored) */
for(i = 1; i 5; i++)
out_be32(pci-pow[i].powar, 0);
for(i = 0; i 3; i++)
out_be32(pci-piw[i].piwar, 0);
/* Setup outbound MEM window */
- for(i = 0; i 3; i++)
- if (hose-mem_resources[i].flags IORESOURCE_MEM){
- resource_size_t pci_addr_start =
-hose-mem_resources[i].start -
-hose-pci_mem_offset;
- pr_debug(PCI MEM resource start 0x%016llx, size
0x%016llx.\n,
- (u64)hose-mem_resources[i].start,
- (u64)hose-mem_resources[i].end
- - (u64)hose-mem_resources[i].start + 1);
- out_be32(pci-pow[i+1].potar, (pci_addr_start 12));
- out_be32(pci-pow[i+1].potear, 0);
- out_be32(pci-pow[i+1].powbar,
- (hose
[PATCH 2/2] POWERPC/fsl-pci: Set relaxed ordering on prefetchable ranges
Provides a small speedup when accessing pefetchable ranges. To indicate
that a memory range is prefetchable, mark it in the dts file with 4200
instead of 0200.
A powepc pci_controller is allowed three memory ranges, any of which may be
prefetchable. However, the PCI-PCI bridge configuration space only has one
field for non-prefetchable memory behind bridge, which has a 32 bit
address, and one field for prefetchable memory behind bridge, which may
have a 64 bit address. These are PCI bus addresses, not CPU physical
addresses.
So really you're only allowed one memory range of each type. And if you
want the range at a PCI address above 32 bits you must make it
prefetchable.
Signed-off-by: Trent Piepho tpie...@freescale.com
---
arch/powerpc/sysdev/fsl_pci.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index 656f914..af0d7e3 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -41,6 +41,9 @@ static int __init setup_one_atmu(struct ccsr_pci __iomem *pci,
pr_debug(PCI MEM resource start 0x%016llx, size 0x%016llx.\n,
(u64)res-start, (u64)size);
+ if (res-flags IORESOURCE_PREFETCH)
+ flags |= 0x1000; /* enable relaxed ordering */
+
for (i = 0; size 0; i++) {
unsigned int bits = min(__ilog2(size),
__ffs(pci_addr | phys_addr));
--
1.5.4.1
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] POWERPC: MTD: Add cached map support to physmap_of MTD driver
The MTD system supports operation where a direct mapped flash chip is
mapped twice. The normal mapping is a standard ioremap(), which is
non-cached and guarded on powerpc. The second mapping is used only for
reads and can be cached and non-guarded. Currently, only the pxa2xx
mapping driver makes use of this feature. This patch adds support to the
physmap_of driver on PPC32 platforms for this cached mapping mode.
Because the flash chip doesn't participate in the cache coherency protocol,
it's necessary to invalidate the cache for parts of flash that are modified
with a program or erase operation. This is platform specific, for instance
the pxa2xx driver uses an ARM specific function. This patch adds
invalidate_dcache_icache_range() for PPC32 and uses it. Because of XIP,
it's entirely possible that the flash might be in the icache(*), so the
existing invalidate_dcache_range() function isn't enough.
Of course, a cached mapping can increase performance if the data is read
from cache instead of flash. But less obvious is that it can provide a
significant performance increase for cold-cache reads that still come from
flash. It allows efficient back-to-back reads and if the flash chip
controller support page burst mode, it allows that to be used as well.
The figures are for *cold-cache* read performance, measured on a Freescale
MPC8572 controlling a Spansion S29GL064N NOR flash chip. With and without
the flash being mapped cached and with and without the localbus controller
being programmed to use page burst mode:
Non-cached, w/o bursts: 13.61 MB/s
Non-cached, w/ bursts: 13.61 MB/s
Cached, w/o bursts: 16.75 MB/s 23% increase
Cached, w/ bursts: 44.79 MB/s 229% increase!
Even without any cache hits, the cached mapping provides a significant
increase in performance via improved bus utilization. Enabling burst
transfers is even more significant.
(*) The MTD device's -point() method, which is the mechanism for
supporting mmap and XIP, only allows for mmapping the uncached region. So
you can't actually XIP anything in the cache. But this could be fixed.
Signed-off-by: Trent Piepho tpie...@freescale.com
---
arch/powerpc/include/asm/cacheflush.h |2 ++
arch/powerpc/kernel/misc_32.S | 21 +
drivers/mtd/maps/physmap_of.c | 20
3 files changed, 43 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/include/asm/cacheflush.h
b/arch/powerpc/include/asm/cacheflush.h
index ba667a3..385c26c 100644
--- a/arch/powerpc/include/asm/cacheflush.h
+++ b/arch/powerpc/include/asm/cacheflush.h
@@ -49,6 +49,8 @@ extern void flush_dcache_range(unsigned long start, unsigned
long stop);
#ifdef CONFIG_PPC32
extern void clean_dcache_range(unsigned long start, unsigned long stop);
extern void invalidate_dcache_range(unsigned long start, unsigned long stop);
+extern void invalidate_dcache_icache_range(unsigned long start,
+ unsigned long stop);
#endif /* CONFIG_PPC32 */
#ifdef CONFIG_PPC64
extern void flush_inval_dcache_range(unsigned long start, unsigned long stop);
diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.S
index d108715..5e6a154 100644
--- a/arch/powerpc/kernel/misc_32.S
+++ b/arch/powerpc/kernel/misc_32.S
@@ -639,6 +639,27 @@ _GLOBAL(invalidate_dcache_range)
blr
/*
+ * Like above, but invalidate both the D-cache and I-cache. Used when a cached
+ * region has been modified from a source that does not participate in the
cache
+ * coherency protocol.
+ *
+ * invalidate_dcache_icache_range(unsigned long start, unsigned long stop)
+ */
+_GLOBAL(invalidate_dcache_icache_range)
+ clrrwi r3, r3, L1_CACHE_SHIFT /* start = ~((1SHIFT)-1) */
+ subfr4,r3,r4
+ addir4,r4,L1_CACHE_BYTES-1
+ srwi. r4,r4,L1_CACHE_SHIFT/* count = (start-stop+BYTES-1)/BYTES */
+ beqlr /* if (!count) return */
+ mtctr r4
+1: dcbi0,r3
+ icbi0,r3
+ addir3,r3,L1_CACHE_BYTES
+ bdnz1b
+ sync/* wait for [id]cbi's to get to ram */
+ blr
+
+/*
* Flush a particular page from the data cache to RAM.
* Note: this is necessary because the instruction cache does *not*
* snoop from the data cache.
diff --git a/drivers/mtd/maps/physmap_of.c b/drivers/mtd/maps/physmap_of.c
index 5fcfec0..e834298 100644
--- a/drivers/mtd/maps/physmap_of.c
+++ b/drivers/mtd/maps/physmap_of.c
@@ -32,6 +32,16 @@ struct of_flash {
#endif
};
+#ifdef CONFIG_PPC32
+#include asm/cacheflush.h
+static void ppc32_inval_cache(struct map_info *map, unsigned long from,
+ ssize_t len)
+{
+ invalidate_dcache_icache_range((unsigned long)map-cached + from,
+ (unsigned long)map-cached + from + len);
+}
+#endif
+
#ifdef CONFIG_MTD_PARTITIONS
#define OF_FLASH_PARTS(info) ((info)-parts)
@@ -106,6 +116,8
Re: [PATCH] POWERPC: MTD: Add cached map support to physmap_of MTD driver
On Mon, 15 Dec 2008, Josh Boyer wrote: Did you actually change anything in this version when compared to the version you sent out last week? If not, is there a reason you sent it again without flagging it as a resend? I sent it out last week? I'm trying to tie up loose ends before I leave and thought this patch hadn't gone out yet. No changes unless there was some kind of last minute spelling fix in the patch comments. At least this one has my non-Freescale address CCed so I'll get replies if there are any. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] POWERPC: MTD: Add cached map support to physmap_of MTD driver
On Tue, 16 Dec 2008, Paul Mackerras wrote: Trent Piepho writes: The MTD system supports operation where a direct mapped flash chip is mapped twice. The normal mapping is a standard ioremap(), which is non-cached and guarded on powerpc. The second mapping is used only for reads and can be cached and non-guarded. Currently, only the pxa2xx mapping driver makes use of this feature. This patch adds support to the physmap_of driver on PPC32 platforms for this cached mapping mode. Note that having two mappings of the same physical address that differ in cacheability is a programming error according to the PowerPC architecture. Do you know that the processor implementations where you want to do this can cope with having two mappings with different cacheability? Good question. It worked for me, but I guess all powerpc32 can't handle it. Shame, as it provides a huge speed up. I suppose an alternative would be to map the chip twice at different physical addresses, by just configuring the chip select to be twice the size it should be, and giving them different cacheability. Or changing the mapping for writes and then changing it back. It wouldn't be necessary to change the whole thing, just the page being written to. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Re: How to support 3GB pci address?
On Sat, 13 Dec 2008, maillist.kernel wrote: Thanks for all the suggestions and Comments! In the system, the total memory size is less than 4GB. I want to know how to map the 3GB pci address to the kernel , and how can my driver access all the pci device. Usually we have only 1GB kernel address, minus the 896MB for memory map, we can only use the 128M for ioremap. I can adjust the PAGE_OFFSET to a lower value, but it's not enough. My contract with Freescale is expiring very soon, so you will probably not be able to reach me at this email address next week. In order to ioremap() 3GB from the kernel, I think what you'll need to do is reduce the user task size and low mem size to less than 1 GB. Set the custom user task to something like 512MB, set PAGE_OFFSET to whatever the user task size is, and set maximum low memory to 384 MB. That should give you 3.25 GB for kernel ioremap()s. If your BARs are 3GB total you'll need a little more than that for other things the kernel will map. Of course, the user task size and lowmem size are now rather small I've tested ioremap() of a 2GB PCI BAR and I know at least that much can work. Current kernels require that PAGE_OFFSET be 256 MB aligned, but I've posted patches that reduce this requirement. They've been ignored so far. One can mmap() a PCI BAR from userspace, in which case the mapping comes out of the max userspace size pool instead of the all ioremap()s pool. The userspace pool is per processes. So while having four kernel drivers each call ioremap(..., 1GB) will never work, it is possible to have four userspace processes each call mmap(/sys/bus/pci.../resource, 1GB) and have it work. There are many pci devices in the system and every pci device has only several tens of MB, so how can I call the mmap(/sys/bus/pci.../resource, 1GB) and how can I use it by my drivers ? There are resource files in sysfs for each PCI BAR. For example, say we have this device: 00:00.0 Host bridge: Advanced Micro Devices [AMD] AMD-760 MP Flags: bus master, 66Mhz, medium devsel, latency 32 Memory at e800 (32-bit, prefetchable) [size=128M] Memory at e700 (32-bit, prefetchable) [size=4K] I/O ports at 1020 [disabled] [size=4] Then in sysfs we will have these files: -rw--- root 128M /sys/bus/pci/devices/:00:00.0/resource0 -rw--- root 4.0K /sys/bus/pci/devices/:00:00.0/resource1 -rw--- root4 /sys/bus/pci/devices/:00:00.0/resource2 A userspace process can use mmap() on them to map the PCI BAR into its address space. Because each process get it's own address space, you could have multiple processes which have mapped a total of more than 4 GB. But this is for userspace processes, not the kernel. The kernel only gets one address space. On Fri, 12 Dec 2008, Kumar Gala wrote: On Dec 12, 2008, at 3:04 AM, Trent Piepho wrote: On Thu, 11 Dec 2008, Kumar Gala wrote: On Dec 11, 2008, at 10:07 PM, Trent Piepho wrote: On Thu, 11 Dec 2008, Kumar Gala wrote: The 36-bit support is current (in tree) in complete. Work is in add swiotlb support to PPC which will generically enable what you Don't the ATMU windows in the pcie controller serve as a IOMMU, making swiotlb unnecessary and wasteful? Nope. You have no way to tell when to switch a window as you have no idea when a device might DMA data. Isn't that what dma_alloc_coherent() and dma_map_single() are for? Nope. How would manipulate the PCI ATMU? Umm, out_be32()? Why would it be any different than other iommu implementations, like the pseries one for example? Just define set a of fsl dma ops that use an inbound ATMU window if they need to. The only issue would be if you have a 32-bit device with multiple concurrent DMA buffers scattered over 32 bits of address space and run out of ATMU windows. But other iommu implementations have that same limitation. You just have to try harder to allocate GFP_DMA memory that doesn't need an ATMU window or create larger contiguous bounce buffer to replace scattered smaller buffers. It sounded like the original poster was talking about having 3GB of PCI BARs. How does swiotlb even enter the picture for that? It wasn't clear how much system memory they wanted. If they can fit their entire memory map for PCI addresses in 4G of address space (this includes all of system DRAM) than they don't need anything special. Why the need to fit the entire PCI memory map into the lower 4G? What issue is there with mapping a PCI BAR above 4G if you have 36-bit support? Putting system memory below 4GB is only an issue if you're talking about DMA. For mapping a PCI BAR, what does it doesn't matter? The problem I see with having large PCI BARs, is that the max userspace process size plus low memory plus all ioremap()s must be less than 4GB. If one wants to call ioremap(..., 3GB), then only 1 GB is left for userspace plus low memory. That's
Re: How to support 3GB pci address?
On Thu, 11 Dec 2008, Kumar Gala wrote: On Dec 11, 2008, at 10:07 PM, Trent Piepho wrote: On Thu, 11 Dec 2008, Kumar Gala wrote: On Dec 11, 2008, at 6:04 AM, maillist.kernel wrote: In the system, the total PCI address needed is about 3GB, so I want to know how to support it in linux. mpc8548 has 36-bit real address, and can support 32GB PCIE address space, but in linux, there is only 1GB kernel space, how to map the 3GB pci address to kernel? Is the 36-bit real address only used to support large memory(4GB) for muti-threads? The 36-bit support is current (in tree) in complete. Work is in progress to add swiotlb support to PPC which will generically enable what you want to accomplish. Don't the ATMU windows in the pcie controller serve as a IOMMU, making swiotlb unnecessary and wasteful? Nope. You have no way to tell when to switch a window as you have no idea when a device might DMA data. Isn't that what dma_alloc_coherent() and dma_map_single() are for? It sounded like the original poster was talking about having 3GB of PCI BARs. How does swiotlb even enter the picture for that? From what I've read about swiotlb, it is a hack that allows one to do DMA with 32-bit PCI devices on 64-bit systems that lack an IOMMU. It reserves a large block of RAM under 32-bits (technically it uses GFP_DMA) and doles this out to drivers that allocate DMA memory. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: How to support 3GB pci address?
On Fri, 12 Dec 2008, Kumar Gala wrote: On Dec 12, 2008, at 3:04 AM, Trent Piepho wrote: On Thu, 11 Dec 2008, Kumar Gala wrote: On Dec 11, 2008, at 10:07 PM, Trent Piepho wrote: On Thu, 11 Dec 2008, Kumar Gala wrote: The 36-bit support is current (in tree) in complete. Work is in add swiotlb support to PPC which will generically enable what you Don't the ATMU windows in the pcie controller serve as a IOMMU, making swiotlb unnecessary and wasteful? Nope. You have no way to tell when to switch a window as you have no idea when a device might DMA data. Isn't that what dma_alloc_coherent() and dma_map_single() are for? Nope. How would manipulate the PCI ATMU? Umm, out_be32()? Why would it be any different than other iommu implementations, like the pseries one for example? Just define set a of fsl dma ops that use an inbound ATMU window if they need to. The only issue would be if you have a 32-bit device with multiple concurrent DMA buffers scattered over 32 bits of address space and run out of ATMU windows. But other iommu implementations have that same limitation. You just have to try harder to allocate GFP_DMA memory that doesn't need an ATMU window or create larger contiguous bounce buffer to replace scattered smaller buffers. It sounded like the original poster was talking about having 3GB of PCI BARs. How does swiotlb even enter the picture for that? It wasn't clear how much system memory they wanted. If they can fit their entire memory map for PCI addresses in 4G of address space (this includes all of system DRAM) than they don't need anything special. Why the need to fit the entire PCI memory map into the lower 4G? What issue is there with mapping a PCI BAR above 4G if you have 36-bit support? Putting system memory below 4GB is only an issue if you're talking about DMA. For mapping a PCI BAR, what does it doesn't matter? The problem I see with having large PCI BARs, is that the max userspace process size plus low memory plus all ioremap()s must be less than 4GB. If one wants to call ioremap(..., 3GB), then only 1 GB is left for userspace plus low memory. That's not very much. One can mmap() a PCI BAR from userspace, in which case the mapping comes out of the max userspace size pool instead of the all ioremap()s pool. The userspace pool is per processes. So while having four kernel drivers each call ioremap(..., 1GB) will never work, it is possible to have four userspace processes each call mmap(/sys/bus/pci.../resource, 1GB) and have it work. From what I've read about swiotlb, it is a hack that allows one to do DMA with 32-bit PCI devices on 64-bit systems that lack an IOMMU. It reserves a large block of RAM under 32-bits (technically it uses GFP_DMA) and doles this out to drivers that allocate DMA memory. correct. It bounce buffers the DMAs to a 32-bit dma'ble region and copies to/from the 32-bit address. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC] Dummy GPIO driver for use with SPI
On Fri, 12 Dec 2008, Anton Vorontsov wrote:
On Fri, Dec 12, 2008 at 11:59:13AM -0500, Steven A. Falco wrote:
What do you think about having a mechanism to specify that some
SPI slaves have a chip select, while others don't have to have a
chip select managed by the SPI subsystem?
Um.. do you know that you can pass '0' as a GPIO?
For example,
spi-controller {
gpios = pio1 1 0 /* cs0 */
0 /* cs1, no GPIO */
pio2 2 0;/* cs2 */
It's ok the that middle specifier is only one word instead of three? Seems
like 0 0 0 would be better, so all the specifiers are the same size.
normal case;
} else if (gpio == -EEXIST) {
Isn't EEXIST (pathname already exists) backward? Seems like ENOENT would
be the right error code. Except that's used for reading past the end...
Maybe a reading past the end should be EINVAL or EBADF?
Or return ENODEV for the 'hole' cell? Or ENOLINK?
EEXIST is for trying to create something that already exists. The 'hole'
is more like trying to follow a broken link or find something that doesn't
exist.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 1/4] leds: Support OpenFirmware led bindings
On Wed, 10 Dec 2008, Anton Vorontsov wrote: +gpio_direction_output(led_dat-gpio, led_dat-active_low); This can fail (yeah, the original code didn't check return value either). I've added a check. +unsigned int flags; I think it would be better to use `enum of_gpio_flags' type here, just for clarity. You're right, I forgot to change this after the of_get_gpio patch was changed. +ret = create_gpio_led(led, pdata-led_data[pdata-num_leds++], + ofdev-dev, NULL); +if (ret 0) of_node_put(np); is missing here. of_node_put(child) you mean. It's easy to forget this when one exits one of these iterators early, since there's no explicit get or put otherwise. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH v2 2/4] leds: Add option to have GPIO LEDs start on
Yes, there is the default-on trigger but there are problems with that.
For one, it's a inefficient way to do it and requires led trigger support
to be compiled in.
But the real reason is that is produces a glitch on the LED. The GPIO is
allocate with the LED *off*, then *later* when then trigger runs it is
turned back on. If the LED was already on via the GPIO's reset default or
action of the firmware, this produces a glitch where the LED goes from on
to off to on. While normally this is fast enough that it wouldn't be
noticeable to a human observer, there are still serious problems.
One is that there may be something else on the GPIO line, like a hardware
alarm or watchdog, that is fast enough to notice the glitch.
Another is that the kernel may panic before the LED is turned back on, thus
hanging with the LED in the wrong state. This is not just speculation, but
actually happened to me with an embedded system that has an LED which
should turn off when the kernel finishes booting, which was left in the
incorrect state due to a bug in the OF LED binding code.
The platform device binding gains a field in the platform data
default_state that controls this. The OpenFirmware binding uses a
property named default-state that can be set to on or off. The
default if the property isn't present is off.
Signed-off-by: Trent Piepho tpie...@freescale.com
Acked-by: Grant Likely grant.lik...@secretlab.ca
---
Documentation/powerpc/dts-bindings/gpio/led.txt |9 -
drivers/leds/leds-gpio.c|8 ++--
include/linux/leds.h|1 +
3 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/Documentation/powerpc/dts-bindings/gpio/led.txt
b/Documentation/powerpc/dts-bindings/gpio/led.txt
index 4fe14de..5df384d 100644
--- a/Documentation/powerpc/dts-bindings/gpio/led.txt
+++ b/Documentation/powerpc/dts-bindings/gpio/led.txt
@@ -16,10 +16,15 @@ LED sub-node properties:
string defining the trigger assigned to the LED. Current triggers are:
backlight - LED will act as a back-light, controlled by the framebuffer
system
-default-on - LED will turn on
+default-on - LED will turn on, but see default-state below
heartbeat - LED double flashes at a load average based rate
ide-disk - LED indicates disk activity
timer - LED flashes at a fixed, configurable rate
+- default-state: (optional) The initial state of the LED. Valid
+ values are on and off. If the LED is already on or off and the
+ default-state property is set the to same value, then no glitch
+ should be produced where the LED momentarily turns off (or on).
+ The default is off if this property is not present.
Examples:
@@ -36,8 +41,10 @@ run-control {
compatible = gpio-leds;
red {
gpios = mpc8572 6 0;
+ default-state = off;
};
green {
gpios = mpc8572 7 0;
+ default-state = on;
};
}
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index be4d6aa..1ad96d2 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -92,9 +92,10 @@ static int __devinit create_gpio_led(const struct gpio_led
*template,
led_dat-cdev.blink_set = gpio_blink_set;
}
led_dat-cdev.brightness_set = gpio_led_set;
- led_dat-cdev.brightness = LED_OFF;
+ led_dat-cdev.brightness = template-default_state ? LED_FULL : LED_OFF;
- ret = gpio_direction_output(led_dat-gpio, led_dat-active_low);
+ ret = gpio_direction_output(led_dat-gpio,
+ led_dat-active_low ^ template-default_state);
if (ret 0)
goto err;
@@ -261,12 +262,15 @@ static int __devinit of_gpio_leds_probe(struct of_device
*ofdev,
memset(led, 0, sizeof(led));
for_each_child_of_node(np, child) {
enum of_gpio_flags flags;
+ const char *state;
led.gpio = of_get_gpio_flags(child, 0, flags);
led.active_low = flags OF_GPIO_ACTIVE_LOW;
led.name = of_get_property(child, label, NULL) ? :
child-name;
led.default_trigger =
of_get_property(child, linux,default-trigger, NULL);
+ state = of_get_property(child, default-state, NULL);
+ led.default_state = state !strcmp(state, on);
ret = create_gpio_led(led, pdata-led_data[pdata-num_leds++],
ofdev-dev, NULL);
diff --git a/include/linux/leds.h b/include/linux/leds.h
index d3a73f5..caa3987 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -138,6 +138,7 @@ struct gpio_led {
const char *default_trigger;
unsignedgpio;
u8 active_low;
+ u8 default_state;
};
struct gpio_led_platform_data {
--
1.5.4.1
[PATCH v2 1/4] leds: Support OpenFirmware led bindings
Add bindings to support LEDs defined as of_platform devices in addition to
the existing bindings for platform devices.
New options in Kconfig allow the platform binding code and/or the
of_platform code to be turned on. The of_platform code is of course only
available on archs that have OF support.
The existing probe and remove methods are refactored to use new functions
create_gpio_led(), to create and register one led, and delete_gpio_led(),
to unregister and free one led. The new probe and remove methods for the
of_platform driver can then share most of the common probe and remove code
with the platform driver.
The suspend and resume methods aren't shared, but they are very short. The
actual led driving code is the same for LEDs created by either binding.
The OF bindings are based on patch by Anton Vorontsov
avoront...@ru.mvista.com. They have been extended to allow multiple LEDs
per device.
Signed-off-by: Trent Piepho tpie...@freescale.com
Acked-by: Grant Likely grant.lik...@secretlab.ca
Acked-by: Sean MacLennan smaclen...@pikatech.com
CC: devicetree-disc...@ozlabs.org
---
Documentation/powerpc/dts-bindings/gpio/led.txt | 46 -
drivers/leds/Kconfig| 21 ++-
drivers/leds/leds-gpio.c| 237 ++-
3 files changed, 250 insertions(+), 54 deletions(-)
diff --git a/Documentation/powerpc/dts-bindings/gpio/led.txt
b/Documentation/powerpc/dts-bindings/gpio/led.txt
index ff51f4c..4fe14de 100644
--- a/Documentation/powerpc/dts-bindings/gpio/led.txt
+++ b/Documentation/powerpc/dts-bindings/gpio/led.txt
@@ -1,15 +1,43 @@
-LED connected to GPIO
+LEDs connected to GPIO lines
Required properties:
-- compatible : should be gpio-led.
-- label : (optional) the label for this LED. If omitted, the label is
+- compatible : should be gpio-leds.
+
+Each LED is represented as a sub-node of the gpio-leds device. Each
+node's name represents the name of the corresponding LED.
+
+LED sub-node properties:
+- gpios : Should specify the LED's GPIO, see Specifying GPIO information
+ for devices in Documentation/powerpc/booting-without-of.txt. Active
+ low LEDs should be indicated using flags in the GPIO specifier.
+- label : (optional) The label for this LED. If omitted, the label is
taken from the node name (excluding the unit address).
-- gpios : should specify LED GPIO.
+- linux,default-trigger : (optional) This parameter, if present, is a
+ string defining the trigger assigned to the LED. Current triggers are:
+backlight - LED will act as a back-light, controlled by the framebuffer
+ system
+default-on - LED will turn on
+heartbeat - LED double flashes at a load average based rate
+ide-disk - LED indicates disk activity
+timer - LED flashes at a fixed, configurable rate
-Example:
+Examples:
-...@0 {
- compatible = gpio-led;
- label = hdd;
- gpios = mcu_pio 0 1;
+leds {
+ compatible = gpio-leds;
+ hdd {
+ label = IDE Activity;
+ gpios = mcu_pio 0 1; /* Active low */
+ linux,default-trigger = ide-disk;
+ };
};
+
+run-control {
+ compatible = gpio-leds;
+ red {
+ gpios = mpc8572 6 0;
+ };
+ green {
+ gpios = mpc8572 7 0;
+ };
+}
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index e7fb7d2..6c6dc96 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -111,7 +111,26 @@ config LEDS_GPIO
help
This option enables support for the LEDs connected to GPIO
outputs. To be useful the particular board must have LEDs
- and they must be connected to the GPIO lines.
+ and they must be connected to the GPIO lines. The LEDs must be
+ defined as platform devices and/or OpenFirmware platform devices.
+ The code to use these bindings can be selected below.
+
+config LEDS_GPIO_PLATFORM
+ bool Platform device bindings for GPIO LEDs
+ depends on LEDS_GPIO
+ default y
+ help
+ Let the leds-gpio driver drive LEDs which have been defined as
+ platform devices. If you don't know what this means, say yes.
+
+config LEDS_GPIO_OF
+ bool OpenFirmware platform device bindings for GPIO LEDs
+ depends on LEDS_GPIO OF_DEVICE
+ default y
+ help
+ Let the leds-gpio driver drive LEDs which have been defined as
+ of_platform devices. For instance, LEDs which are listed in a dts
+ file.
config LEDS_HP_DISK
tristate LED Support for disk protection LED on HP notebooks
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index b13bd29..be4d6aa 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -3,6 +3,7 @@
*
* Copyright (C) 2007 8D Technologies inc.
* Raphael Assenat r...@8d.com
+ * Copyright (C) 2008 Freescale Semiconductor, Inc.
*
* This program is free software; you can
[PATCH v2 3/4] leds: Let GPIO LEDs keep their current state
Let GPIO LEDs get their initial value from whatever the current state of
the GPIO line is. On some systems the LEDs are put into some state by the
firmware or hardware before Linux boots, and it is desired to have them
keep this state which is otherwise unknown to Linux.
This requires that the underlying GPIO driver support reading the value of
output GPIOs. Some drivers support this and some do not.
The platform data for the platform device binding gets a new field
'keep_state' which turns this on. keep_state overrides default_state.
For the OpenFirmware bindings, the default-state property gains a new
allowable setting keep.
Signed-off-by: Trent Piepho tpie...@freescale.com
Acked-by: Grant Likely grant.lik...@secretlab.ca
---
Documentation/powerpc/dts-bindings/gpio/led.txt | 16
drivers/leds/leds-gpio.c| 12
include/linux/leds.h|3 ++-
3 files changed, 22 insertions(+), 9 deletions(-)
diff --git a/Documentation/powerpc/dts-bindings/gpio/led.txt
b/Documentation/powerpc/dts-bindings/gpio/led.txt
index 5df384d..064db92 100644
--- a/Documentation/powerpc/dts-bindings/gpio/led.txt
+++ b/Documentation/powerpc/dts-bindings/gpio/led.txt
@@ -21,10 +21,12 @@ LED sub-node properties:
ide-disk - LED indicates disk activity
timer - LED flashes at a fixed, configurable rate
- default-state: (optional) The initial state of the LED. Valid
- values are on and off. If the LED is already on or off and the
- default-state property is set the to same value, then no glitch
- should be produced where the LED momentarily turns off (or on).
- The default is off if this property is not present.
+ values are on, off, and keep. If the LED is already on or off
+ and the default-state property is set the to same value, then no
+ glitch should be produced where the LED momentarily turns off (or
+ on). The keep setting will keep the LED at whatever its current
+ state is, without producing a glitch. The default is off if this
+ property is not present.
Examples:
@@ -35,6 +37,12 @@ leds {
gpios = mcu_pio 0 1; /* Active low */
linux,default-trigger = ide-disk;
};
+
+ fault {
+ gpios = mcu_pio 1 0;
+ /* Keep LED on if BIOS detected hardware fault */
+ default-state = keep;
+ };
};
run-control {
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index 1ad96d2..bb9d9ff 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -76,7 +76,7 @@ static int __devinit create_gpio_led(const struct gpio_led
*template,
struct gpio_led_data *led_dat, struct device *parent,
int (*blink_set)(unsigned, unsigned long *, unsigned long *))
{
- int ret;
+ int ret, state;
ret = gpio_request(template-gpio, template-name);
if (ret 0)
@@ -92,10 +92,13 @@ static int __devinit create_gpio_led(const struct gpio_led
*template,
led_dat-cdev.blink_set = gpio_blink_set;
}
led_dat-cdev.brightness_set = gpio_led_set;
- led_dat-cdev.brightness = template-default_state ? LED_FULL : LED_OFF;
+ if (template-keep_state)
+ state = !!gpio_get_value(led_dat-gpio) ^ led_dat-active_low;
+ else
+ state = template-default_state;
+ led_dat-cdev.brightness = state ? LED_FULL : LED_OFF;
- ret = gpio_direction_output(led_dat-gpio,
- led_dat-active_low ^ template-default_state);
+ ret = gpio_direction_output(led_dat-gpio, led_dat-active_low ^ state);
if (ret 0)
goto err;
@@ -271,6 +274,7 @@ static int __devinit of_gpio_leds_probe(struct of_device
*ofdev,
of_get_property(child, linux,default-trigger, NULL);
state = of_get_property(child, default-state, NULL);
led.default_state = state !strcmp(state, on);
+ led.keep_state = state !strcmp(state, keep);
ret = create_gpio_led(led, pdata-led_data[pdata-num_leds++],
ofdev-dev, NULL);
diff --git a/include/linux/leds.h b/include/linux/leds.h
index caa3987..c51b625 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -138,7 +138,8 @@ struct gpio_led {
const char *default_trigger;
unsignedgpio;
u8 active_low;
- u8 default_state;
+ u8 default_state; /* 0 = off, 1 = on */
+ u8 keep_state; /* overrides default_state */
};
struct gpio_led_platform_data {
--
1.5.4.1
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH v2 4/4] leds: Use tristate property in platform data
Replace the two boolean properties default_state and keep_state with a
single tristate property that can be set to
LEDS_GPIO_DEFSTATE_(ON|OFF|KEEP). This ends up being more complicated,
requires more code, and makes developers remember not just the name of the
field to set but also the symbolic constant to set it to. Yet despite
these shortcomings it remains more popular.
Signed-off-by: Trent Piepho tpie...@freescale.com
---
drivers/leds/leds-gpio.c | 15 +++
include/linux/leds.h |7 +--
2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index bb9d9ff..2d1b71f 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -92,10 +92,10 @@ static int __devinit create_gpio_led(const struct gpio_led
*template,
led_dat-cdev.blink_set = gpio_blink_set;
}
led_dat-cdev.brightness_set = gpio_led_set;
- if (template-keep_state)
+ if (template-default_state == LEDS_GPIO_DEFSTATE_KEEP)
state = !!gpio_get_value(led_dat-gpio) ^ led_dat-active_low;
else
- state = template-default_state;
+ state = (template-default_state == LEDS_GPIO_DEFSTATE_ON);
led_dat-cdev.brightness = state ? LED_FULL : LED_OFF;
ret = gpio_direction_output(led_dat-gpio, led_dat-active_low ^ state);
@@ -273,8 +273,15 @@ static int __devinit of_gpio_leds_probe(struct of_device
*ofdev,
led.default_trigger =
of_get_property(child, linux,default-trigger, NULL);
state = of_get_property(child, default-state, NULL);
- led.default_state = state !strcmp(state, on);
- led.keep_state = state !strcmp(state, keep);
+ if (state) {
+ if (!strcmp(state, keep)) {
+ led.default_state = LEDS_GPIO_DEFSTATE_KEEP;
+ } else if(!strcmp(state, on)) {
+ led.default_state = LEDS_GPIO_DEFSTATE_ON;
+ } else {
+ led.default_state = LEDS_GPIO_DEFSTATE_OFF;
+ }
+ }
ret = create_gpio_led(led, pdata-led_data[pdata-num_leds++],
ofdev-dev, NULL);
diff --git a/include/linux/leds.h b/include/linux/leds.h
index c51b625..f4a125c 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -138,9 +138,12 @@ struct gpio_led {
const char *default_trigger;
unsignedgpio;
u8 active_low;
- u8 default_state; /* 0 = off, 1 = on */
- u8 keep_state; /* overrides default_state */
+ u8 default_state;
+ /* default_state should be one of LEDS_GPIO_DEFSTATE_(ON|OFF|KEEP) */
};
+#define LEDS_GPIO_DEFSTATE_OFF 0
+#define LEDS_GPIO_DEFSTATE_ON 1
+#define LEDS_GPIO_DEFSTATE_KEEP2
struct gpio_led_platform_data {
int num_leds;
--
1.5.4.1
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: How to support 3GB pci address?
On Thu, 11 Dec 2008, Kumar Gala wrote: On Dec 11, 2008, at 6:04 AM, maillist.kernel wrote: In the system, the total PCI address needed is about 3GB, so I want to know how to support it in linux. mpc8548 has 36-bit real address, and can support 32GB PCIE address space, but in linux, there is only 1GB kernel space, how to map the 3GB pci address to kernel? Is the 36-bit real address only used to support large memory(4GB) for muti-threads? The 36-bit support is current (in tree) in complete. Work is in progress to add swiotlb support to PPC which will generically enable what you want to accomplish. Don't the ATMU windows in the pcie controller serve as a IOMMU, making swiotlb unnecessary and wasteful? ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] fsl-booke: declare tlbcam_index and num_tlbcam_entries for use in c file
On Wed, 10 Dec 2008, Liu Yu wrote: KVM on E500 platform currently utilize TLB1 entries without bothering host, that is using unused TLB1 entries. So, KVM needs to read tlbcam_index and num_tlbcam_entries to know exactly which TLB1 entry is unused. Do you need really need num_tlbcam_entries? It's completely unused and I've already sent out a patch to remove it. If you want to know how big TLB1 is, just read it from SPRN_TLB1CFG. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH 3/4] leds: Let GPIO LEDs keep their current state
Let GPIO LEDs get their initial value from whatever the current state of
the GPIO line is. On some systems the LEDs are put into some state by the
firmware or hardware before Linux boots, and it is desired to have them
keep this state which is otherwise unknown to Linux.
This requires that the underlying GPIO driver support reading the value of
output GPIOs. Some drivers support this and some do not.
The platform data for the platform device binding gets a new field
'keep_state' which turns this on. keep_state overrides default_state.
For the OpenFirmware bindings, the default-state property gains a new
allowable setting keep.
Signed-off-by: Trent Piepho [EMAIL PROTECTED]
Acked-by: Grant Likely [EMAIL PROTECTED]
---
Documentation/powerpc/dts-bindings/gpio/led.txt | 16
drivers/leds/leds-gpio.c| 12
include/linux/leds.h|3 ++-
3 files changed, 22 insertions(+), 9 deletions(-)
diff --git a/Documentation/powerpc/dts-bindings/gpio/led.txt
b/Documentation/powerpc/dts-bindings/gpio/led.txt
index 5df384d..064db92 100644
--- a/Documentation/powerpc/dts-bindings/gpio/led.txt
+++ b/Documentation/powerpc/dts-bindings/gpio/led.txt
@@ -21,10 +21,12 @@ LED sub-node properties:
ide-disk - LED indicates disk activity
timer - LED flashes at a fixed, configurable rate
- default-state: (optional) The initial state of the LED. Valid
- values are on and off. If the LED is already on or off and the
- default-state property is set the to same value, then no glitch
- should be produced where the LED momentarily turns off (or on).
- The default is off if this property is not present.
+ values are on, off, and keep. If the LED is already on or off
+ and the default-state property is set the to same value, then no
+ glitch should be produced where the LED momentarily turns off (or
+ on). The keep setting will keep the LED at whatever its current
+ state is, without producing a glitch. The default is off if this
+ property is not present.
Examples:
@@ -35,6 +37,12 @@ leds {
gpios = mcu_pio 0 1; /* Active low */
linux,default-trigger = ide-disk;
};
+
+ fault {
+ gpios = mcu_pio 1 0;
+ /* Keep LED on if BIOS detected hardware fault */
+ default-state = keep;
+ };
};
run-control {
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index 21084a3..e33cdd3 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -76,7 +76,7 @@ static int __devinit create_gpio_led(const struct gpio_led
*template,
struct gpio_led_data *led_dat, struct device *parent,
int (*blink_set)(unsigned, unsigned long *, unsigned long *))
{
- int ret;
+ int ret, state;
ret = gpio_request(template-gpio, template-name);
if (ret 0)
@@ -92,10 +92,13 @@ static int __devinit create_gpio_led(const struct gpio_led
*template,
led_dat-cdev.blink_set = gpio_blink_set;
}
led_dat-cdev.brightness_set = gpio_led_set;
- led_dat-cdev.brightness = template-default_state ? LED_FULL : LED_OFF;
+ if (template-keep_state)
+ state = !!gpio_get_value(led_dat-gpio) ^ led_dat-active_low;
+ else
+ state = template-default_state;
+ led_dat-cdev.brightness = state ? LED_FULL : LED_OFF;
- gpio_direction_output(led_dat-gpio,
- led_dat-active_low ^ template-default_state);
+ gpio_direction_output(led_dat-gpio, led_dat-active_low ^ state);
INIT_WORK(led_dat-work, gpio_led_work);
@@ -266,6 +269,7 @@ static int __devinit of_gpio_leds_probe(struct of_device
*ofdev,
of_get_property(child, linux,default-trigger, NULL);
state = of_get_property(child, default-state, NULL);
led.default_state = state !strcmp(state, on);
+ led.keep_state = state !strcmp(state, keep);
ret = create_gpio_led(led, pdata-led_data[pdata-num_leds++],
ofdev-dev, NULL);
diff --git a/include/linux/leds.h b/include/linux/leds.h
index caa3987..c51b625 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -138,7 +138,8 @@ struct gpio_led {
const char *default_trigger;
unsignedgpio;
u8 active_low;
- u8 default_state;
+ u8 default_state; /* 0 = off, 1 = on */
+ u8 keep_state; /* overrides default_state */
};
struct gpio_led_platform_data {
--
1.5.4.1
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH 1/4] leds: Support OpenFirmware led bindings
Add bindings to support LEDs defined as of_platform devices in addition to
the existing bindings for platform devices.
New options in Kconfig allow the platform binding code and/or the
of_platform code to be turned on. The of_platform code is of course only
available on archs that have OF support.
The existing probe and remove methods are refactored to use new functions
create_gpio_led(), to create and register one led, and delete_gpio_led(),
to unregister and free one led. The new probe and remove methods for the
of_platform driver can then share most of the common probe and remove code
with the platform driver.
The suspend and resume methods aren't shared, but they are very short. The
actual led driving code is the same for LEDs created by either binding.
The OF bindings are based on patch by Anton Vorontsov
[EMAIL PROTECTED]. They have been extended to allow multiple LEDs
per device.
Signed-off-by: Trent Piepho [EMAIL PROTECTED]
Acked-by: Grant Likely [EMAIL PROTECTED]
Acked-by: Sean MacLennan [EMAIL PROTECTED]
CC: [EMAIL PROTECTED]
---
Documentation/powerpc/dts-bindings/gpio/led.txt | 46 -
drivers/leds/Kconfig| 21 ++-
drivers/leds/leds-gpio.c| 230 ++-
3 files changed, 243 insertions(+), 54 deletions(-)
diff --git a/Documentation/powerpc/dts-bindings/gpio/led.txt
b/Documentation/powerpc/dts-bindings/gpio/led.txt
index ff51f4c..4fe14de 100644
--- a/Documentation/powerpc/dts-bindings/gpio/led.txt
+++ b/Documentation/powerpc/dts-bindings/gpio/led.txt
@@ -1,15 +1,43 @@
-LED connected to GPIO
+LEDs connected to GPIO lines
Required properties:
-- compatible : should be gpio-led.
-- label : (optional) the label for this LED. If omitted, the label is
+- compatible : should be gpio-leds.
+
+Each LED is represented as a sub-node of the gpio-leds device. Each
+node's name represents the name of the corresponding LED.
+
+LED sub-node properties:
+- gpios : Should specify the LED's GPIO, see Specifying GPIO information
+ for devices in Documentation/powerpc/booting-without-of.txt. Active
+ low LEDs should be indicated using flags in the GPIO specifier.
+- label : (optional) The label for this LED. If omitted, the label is
taken from the node name (excluding the unit address).
-- gpios : should specify LED GPIO.
+- linux,default-trigger : (optional) This parameter, if present, is a
+ string defining the trigger assigned to the LED. Current triggers are:
+backlight - LED will act as a back-light, controlled by the framebuffer
+ system
+default-on - LED will turn on
+heartbeat - LED double flashes at a load average based rate
+ide-disk - LED indicates disk activity
+timer - LED flashes at a fixed, configurable rate
-Example:
+Examples:
[EMAIL PROTECTED] {
- compatible = gpio-led;
- label = hdd;
- gpios = mcu_pio 0 1;
+leds {
+ compatible = gpio-leds;
+ hdd {
+ label = IDE Activity;
+ gpios = mcu_pio 0 1; /* Active low */
+ linux,default-trigger = ide-disk;
+ };
};
+
+run-control {
+ compatible = gpio-leds;
+ red {
+ gpios = mpc8572 6 0;
+ };
+ green {
+ gpios = mpc8572 7 0;
+ };
+}
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index e7fb7d2..6c6dc96 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -111,7 +111,26 @@ config LEDS_GPIO
help
This option enables support for the LEDs connected to GPIO
outputs. To be useful the particular board must have LEDs
- and they must be connected to the GPIO lines.
+ and they must be connected to the GPIO lines. The LEDs must be
+ defined as platform devices and/or OpenFirmware platform devices.
+ The code to use these bindings can be selected below.
+
+config LEDS_GPIO_PLATFORM
+ bool Platform device bindings for GPIO LEDs
+ depends on LEDS_GPIO
+ default y
+ help
+ Let the leds-gpio driver drive LEDs which have been defined as
+ platform devices. If you don't know what this means, say yes.
+
+config LEDS_GPIO_OF
+ bool OpenFirmware platform device bindings for GPIO LEDs
+ depends on LEDS_GPIO OF_DEVICE
+ default y
+ help
+ Let the leds-gpio driver drive LEDs which have been defined as
+ of_platform devices. For instance, LEDs which are listed in a dts
+ file.
config LEDS_HP_DISK
tristate LED Support for disk protection LED on HP notebooks
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index b13bd29..107b958 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -3,6 +3,7 @@
*
* Copyright (C) 2007 8D Technologies inc.
* Raphael Assenat [EMAIL PROTECTED]
+ * Copyright (C) 2008 Freescale Semiconductor, Inc.
*
* This program is free software; you can redistribute
[PATCH 4/4] leds: Use tristate property in platform data
Replace the two boolean properties default_state and keep_state with a
single tristate property that can be set to
LEDS_GPIO_DEFSTATE_(ON|OFF|KEEP). This ends up being more complicated,
requires more code, and makes developers remember not just the name of the
field to set but also the symbolic constant to set it to. Yet despite
these shortcomings it remains more popular.
Signed-off-by: Trent Piepho [EMAIL PROTECTED]
---
drivers/leds/leds-gpio.c | 15 +++
include/linux/leds.h |7 +--
2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index e33cdd3..47da332 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -92,10 +92,10 @@ static int __devinit create_gpio_led(const struct gpio_led
*template,
led_dat-cdev.blink_set = gpio_blink_set;
}
led_dat-cdev.brightness_set = gpio_led_set;
- if (template-keep_state)
+ if (template-default_state == LEDS_GPIO_DEFSTATE_KEEP)
state = !!gpio_get_value(led_dat-gpio) ^ led_dat-active_low;
else
- state = template-default_state;
+ state = (template-default_state == LEDS_GPIO_DEFSTATE_ON);
led_dat-cdev.brightness = state ? LED_FULL : LED_OFF;
gpio_direction_output(led_dat-gpio, led_dat-active_low ^ state);
@@ -268,8 +268,15 @@ static int __devinit of_gpio_leds_probe(struct of_device
*ofdev,
led.default_trigger =
of_get_property(child, linux,default-trigger, NULL);
state = of_get_property(child, default-state, NULL);
- led.default_state = state !strcmp(state, on);
- led.keep_state = state !strcmp(state, keep);
+ if (state) {
+ if (!strcmp(state, keep)) {
+ led.default_state = LEDS_GPIO_DEFSTATE_KEEP;
+ } else if(!strcmp(state, on)) {
+ led.default_state = LEDS_GPIO_DEFSTATE_ON;
+ } else {
+ led.default_state = LEDS_GPIO_DEFSTATE_OFF;
+ }
+ }
ret = create_gpio_led(led, pdata-led_data[pdata-num_leds++],
ofdev-dev, NULL);
diff --git a/include/linux/leds.h b/include/linux/leds.h
index c51b625..f4a125c 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -138,9 +138,12 @@ struct gpio_led {
const char *default_trigger;
unsignedgpio;
u8 active_low;
- u8 default_state; /* 0 = off, 1 = on */
- u8 keep_state; /* overrides default_state */
+ u8 default_state;
+ /* default_state should be one of LEDS_GPIO_DEFSTATE_(ON|OFF|KEEP) */
};
+#define LEDS_GPIO_DEFSTATE_OFF 0
+#define LEDS_GPIO_DEFSTATE_ON 1
+#define LEDS_GPIO_DEFSTATE_KEEP2
struct gpio_led_platform_data {
int num_leds;
--
1.5.4.1
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH 2/4] leds: Add option to have GPIO LEDs start on
Yes, there is the default-on trigger but there are problems with that.
For one, it's a inefficient way to do it and requires led trigger support
to be compiled in.
But the real reason is that is produces a glitch on the LED. The GPIO is
allocate with the LED *off*, then *later* when then trigger runs it is
turned back on. If the LED was already on via the GPIO's reset default or
action of the firmware, this produces a glitch where the LED goes from on
to off to on. While normally this is fast enough that it wouldn't be
noticeable to a human observer, there are still serious problems.
One is that there may be something else on the GPIO line, like a hardware
alarm or watchdog, that is fast enough to notice the glitch.
Another is that the kernel may panic before the LED is turned back on, thus
hanging with the LED in the wrong state. This is not just speculation, but
actually happened to me with an embedded system that has an LED which
should turn off when the kernel finishes booting, which was left in the
incorrect state due to a bug in the OF LED binding code.
The platform device binding gains a field in the platform data
default_state that controls this. The OpenFirmware binding uses a
property named default-state that can be set to on or off. The
default if the property isn't present is off.
Signed-off-by: Trent Piepho [EMAIL PROTECTED]
Acked-by: Grant Likely [EMAIL PROTECTED]
---
Documentation/powerpc/dts-bindings/gpio/led.txt |9 -
drivers/leds/leds-gpio.c|8 ++--
include/linux/leds.h|1 +
3 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/Documentation/powerpc/dts-bindings/gpio/led.txt
b/Documentation/powerpc/dts-bindings/gpio/led.txt
index 4fe14de..5df384d 100644
--- a/Documentation/powerpc/dts-bindings/gpio/led.txt
+++ b/Documentation/powerpc/dts-bindings/gpio/led.txt
@@ -16,10 +16,15 @@ LED sub-node properties:
string defining the trigger assigned to the LED. Current triggers are:
backlight - LED will act as a back-light, controlled by the framebuffer
system
-default-on - LED will turn on
+default-on - LED will turn on, but see default-state below
heartbeat - LED double flashes at a load average based rate
ide-disk - LED indicates disk activity
timer - LED flashes at a fixed, configurable rate
+- default-state: (optional) The initial state of the LED. Valid
+ values are on and off. If the LED is already on or off and the
+ default-state property is set the to same value, then no glitch
+ should be produced where the LED momentarily turns off (or on).
+ The default is off if this property is not present.
Examples:
@@ -36,8 +41,10 @@ run-control {
compatible = gpio-leds;
red {
gpios = mpc8572 6 0;
+ default-state = off;
};
green {
gpios = mpc8572 7 0;
+ default-state = on;
};
}
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index 107b958..21084a3 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -92,9 +92,10 @@ static int __devinit create_gpio_led(const struct gpio_led
*template,
led_dat-cdev.blink_set = gpio_blink_set;
}
led_dat-cdev.brightness_set = gpio_led_set;
- led_dat-cdev.brightness = LED_OFF;
+ led_dat-cdev.brightness = template-default_state ? LED_FULL : LED_OFF;
- gpio_direction_output(led_dat-gpio, led_dat-active_low);
+ gpio_direction_output(led_dat-gpio,
+ led_dat-active_low ^ template-default_state);
INIT_WORK(led_dat-work, gpio_led_work);
@@ -256,12 +257,15 @@ static int __devinit of_gpio_leds_probe(struct of_device
*ofdev,
memset(led, 0, sizeof(led));
for_each_child_of_node(np, child) {
unsigned int flags;
+ const char *state;
led.gpio = of_get_gpio_flags(child, 0, flags);
led.active_low = flags OF_GPIO_ACTIVE_LOW;
led.name = of_get_property(child, label, NULL) ? :
child-name;
led.default_trigger =
of_get_property(child, linux,default-trigger, NULL);
+ state = of_get_property(child, default-state, NULL);
+ led.default_state = state !strcmp(state, on);
ret = create_gpio_led(led, pdata-led_data[pdata-num_leds++],
ofdev-dev, NULL);
diff --git a/include/linux/leds.h b/include/linux/leds.h
index d3a73f5..caa3987 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -138,6 +138,7 @@ struct gpio_led {
const char *default_trigger;
unsignedgpio;
u8 active_low;
+ u8 default_state;
};
struct gpio_led_platform_data {
--
1.5.4.1
___
Linuxppc-dev mailing list
Re: [RFC/PATCH 1/2] powerpc: Rework usage of _PAGE_COHERENT/NO_CACHE/GUARDED
On Wed, 10 Dec 2008, Benjamin Herrenschmidt wrote:
This changes the logic so that instead, the PTE now contains
_PAGE_COHERENT for all normal RAM pages tha have I = 0. The hash
code clears it if the feature bit is not set.
Why not check the feature bit when the PTE is made and unset _PAGE_COHERENT
at that point? In fact, could you do something like:
#if defined(CONFIG_SMP) ||
#define _PAGE_BASE (_PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_COHERENT)
#else
#define _PAGE_BASE (_PAGE_PRESENT | _PAGE_ACCESSED)
#endif
I haven't touched at the FSL BookE code yet. It may need to selectively
clear M in the TLB miss handler ... or not. Depends what the impact of
M on non-SMP E5xx setup is. I also didn't bother to clear it on 440 because
it just has no effect (ie, it won't slow things down).
I've been told that setting M on non-SMP will slows things down. But
couldn't you just change _PAGE_BASE on non-SMP instead of clearning it in
the miss handler?
for_each_pci_dev(pdev) {
for (i = 0; i = PCI_ROM_RESOURCE; i++) {
struct resource *rp = pdev-resource[i];
@@ -422,14 +417,14 @@ pgprot_t pci_phys_mem_access_prot(struct
}
if (found) {
if (found-flags IORESOURCE_PREFETCH)
- prot = ~_PAGE_GUARDED;
+ prot = pgprot_noncached_wc(prot);
pci_dev_put(pdev);
}
I have a patch to remove this IORESOURCE_PREFETCH hack. The current kernel
creates two files, resourceN and resourceN_wc, for prefetchable BARs to
allow the user to choose what mode to use.
Though I want to be able to map PCI resources as cached too. I'm not sure
what a good way to add that is. Yet another resource file? The first
proposal for allowing WC mappings was to add support for ioctl() on sysfs
attributes. Then one could use ioctl() after opening the resource file to
specify what mode to map it with. No one liked ioctl(), but clearly the
current method doesn't scale well to all 32 combinations of the WIMGE bits.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC/PATCH 1/2] powerpc: Rework usage of _PAGE_COHERENT/NO_CACHE/GUARDED
On Thu, 11 Dec 2008, Benjamin Herrenschmidt wrote: On Wed, 2008-12-10 at 11:33 -0800, Trent Piepho wrote: On Wed, 10 Dec 2008, Benjamin Herrenschmidt wrote: This changes the logic so that instead, the PTE now contains _PAGE_COHERENT for all normal RAM pages tha have I = 0. The hash code clears it if the feature bit is not set. Why not check the feature bit when the PTE is made and unset _PAGE_COHERENT at that point? In fact, could you do something like: Not sure what you mean. Inside set_pte_at ? Well, the one line of asm is going to be in the noise in the hash code, I'm just flipping an existing condition. I like the PTE to represent whether it's supposed to be a coherent page. In the code that does the mapping. It's a lot cheaper to figure out if _PAGE_COHERENT is needed once per mapping instead of per page per fault. It sounds like getting it right is a lot more complicated than just one instruction. No M bit for non-SMP, except for some 74xx, or if a MPC107 bridge is used, which should be determined at runtime. And does the MPC107 thing apply to all pages or just those PCI memory behind the bridge? Or DMA? I've been told that setting M on non-SMP will slows things down. But couldn't you just change _PAGE_BASE on non-SMP instead of clearning it in the miss handler? Well, because we need it set on non SMP on some 74xx.. maybe we can have it set in PAGE_BASE only if CONFIG_SMP and CONFIG_6xx ? That's what I was thinking, set it in page base for SMP and other instances when we know it's necessary at compile time. If/when there is a runtime check, then it would be lot easier to put that check in the code that made the mapping instead of the miss handler. I have a patch to remove this IORESOURCE_PREFETCH hack. The current kernel creates two files, resourceN and resourceN_wc, for prefetchable BARs to allow the user to choose what mode to use. Ah ? That's new ? I missed it. Has X been updated to use them ? If not, keep the hack for a little while more :-) It's rather new so I bet X servers that use it aren't widely deployed yet. commit 45aec1ae72fc592f231e9e73ed9ed4d10cfbc0b5 Author: [EMAIL PROTECTED] [EMAIL PROTECTED] Date: Tue Mar 18 17:00:22 2008 -0700 x86: PAT export resource_wc in pci sysfs Patch title is somewhat misleading, as it doesn't touch any x86 specific code. And people complain when I used booke instead of fsl-booke... like I want to make it any easier to have patches ignored. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] Introduce ppc_pci_flags accessors
On Wed, 10 Dec 2008, Josh Boyer wrote:
On Thu, 11 Dec 2008 10:46:28 +1100
+#ifdef CONFIG_PCI
+extern unsigned int ppc_pci_flags;
+#define ppc_pci_set_flags(flags) ppc_pci_flags = (flags)
+#define ppc_pci_add_flags(flags) ppc_pci_flags |= (flags)
+#define ppc_pci_flag_is_set(flag) (ppc_pci_flags (flag))
+#else
+#define ppc_pci_set_flags(flags) do {} while (0)
+#define ppc_pci_add_flags(flags) do {} while (0)
+#define ppc_pci_flag_is_set(flag) (0)
+#endif
I hate to be picky, but I don't see any reason why these shouldn't be
static inlines.
There's a perfectly good reason. I AM LAZY.
That aside, it doesn't matter to me either way. If the general idea
seems fine and the naming of the functions is acceptable, I'd be happy
to respin.
If were allowed to be picky, I think ppc_pci_has_flag() is a better name
than ppc_pci_flag_is_set(). Matches the other function names better, and a
quick grep of the kernel source shows bar_has_foo() is much more common
than bar_foo_is_set().
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 4/4] leds: Let GPIO LEDs keep their current state
On Wed, 3 Dec 2008, Richard Purdie wrote:
On Sun, 2008-11-23 at 13:31 +0100, Pavel Machek wrote:
On Thu 2008-11-20 17:05:56, Trent Piepho wrote:
I thought of that, but it ends up being more complex. Instead of just
using:
static const struct gpio_led myled = {
.name = something,
.keep_state = 1,
}
You'd do something like this:
.default_state = LEDS_GPIO_DEFSTATE_KEEP,
Is that better?
Yes.
Yes, agreed, much better.
Oh very well, I'll change it. But I reserve the right to make a sarcastic
commit message.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] powerpc/85xx: Add support for SMP initialization
On Tue, 2 Dec 2008, Kumar Gala wrote: Added 85xx specifc smp_ops structure. We use ePAPR style boot release and the MPIC for IPIs at this point. Additionally added routines for secondary cpu entry and initializtion. @@ -740,6 +750,9 @@ finish_tlb_load: #else rlwimi r12, r11, 26, 27, 31/* extract WIMGE from pte */ #endif +#ifdef CONFIG_SMP + ori r12, r12, MAS2_M +#endif mtspr SPRN_MAS2, r12 Wouldn't it be more efficient to set _PAGE_COHERENT when the pte is created vs setting MAS2_M each time it's loaded? Is it correct to set MAS2_M for all pages, even uncached ones? The code for ioremap() has this: /* Non-cacheable page cannot be coherent */ if (flags _PAGE_NO_CACHE) flags = ~_PAGE_COHERENT; It seems odd that ioremap would explictly unset _PAGE_COHERENT when the code that sets the tlb will just force it back on. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH 1/5] powerpc: booke: Don't hard-code size of struct tlbcam
Some assembly code in head_fsl_booke.S hard-coded the size of struct tlbcam
to 20 when it indexed the TLBCAM table. Anyone changing the size of struct
tlbcam would not know to expect that.
The kernel already has a system to get the size of C structures into
assembly language files, asm-offsets, so let's use it.
The definition of the struct gets moved to a header, so that asm-offsets.c
can include it.
Signed-off-by: Trent Piepho [EMAIL PROTECTED]
---
arch/powerpc/kernel/asm-offsets.c|8
arch/powerpc/kernel/head_fsl_booke.S |2 +-
arch/powerpc/mm/fsl_booke_mmu.c |8 +---
arch/powerpc/mm/mmu_decl.h |9 +
4 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/kernel/asm-offsets.c
b/arch/powerpc/kernel/asm-offsets.c
index 050abfd..4d62a8d 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -56,6 +56,10 @@
#include head_booke.h
#endif
+#if defined(CONFIG_FSL_BOOKE)
+#include ../mm/mmu_decl.h
+#endif
+
int main(void)
{
DEFINE(THREAD, offsetof(struct task_struct, thread));
@@ -271,6 +275,10 @@ int main(void)
DEFINE(SAVED_KSP_LIMIT, STACK_INT_FRAME_SIZE+offsetof(struct
exception_regs, saved_ksp_limit));
#endif
+#ifdef CONFIG_FSL_BOOKE
+ DEFINE(TLBCAM_SIZE, sizeof(struct tlbcam));
+#endif
+
DEFINE(CLONE_VM, CLONE_VM);
DEFINE(CLONE_UNTRACED, CLONE_UNTRACED);
diff --git a/arch/powerpc/kernel/head_fsl_booke.S
b/arch/powerpc/kernel/head_fsl_booke.S
index 9a4639c..c591acb 100644
--- a/arch/powerpc/kernel/head_fsl_booke.S
+++ b/arch/powerpc/kernel/head_fsl_booke.S
@@ -909,7 +909,7 @@ KernelSPE:
_GLOBAL(loadcam_entry)
lis r4,[EMAIL PROTECTED]
addir4,r4,[EMAIL PROTECTED]
- mulli r5,r3,20
+ mulli r5,r3,TLBCAM_SIZE
add r3,r5,r4
lwz r4,0(r3)
mtspr SPRN_MAS0,r4
diff --git a/arch/powerpc/mm/fsl_booke_mmu.c b/arch/powerpc/mm/fsl_booke_mmu.c
index 23cee39..c9ee59a 100644
--- a/arch/powerpc/mm/fsl_booke_mmu.c
+++ b/arch/powerpc/mm/fsl_booke_mmu.c
@@ -61,13 +61,7 @@ static unsigned long __cam0, __cam1, __cam2;
#define NUM_TLBCAMS(16)
-struct tlbcam {
- u32 MAS0;
- u32 MAS1;
- u32 MAS2;
- u32 MAS3;
- u32 MAS7;
-} TLBCAM[NUM_TLBCAMS];
+struct tlbcam TLBCAM[NUM_TLBCAMS];
struct tlbcamrange {
unsigned long start;
diff --git a/arch/powerpc/mm/mmu_decl.h b/arch/powerpc/mm/mmu_decl.h
index fab3cfa..f0d0aae 100644
--- a/arch/powerpc/mm/mmu_decl.h
+++ b/arch/powerpc/mm/mmu_decl.h
@@ -27,6 +27,15 @@ extern void hash_preload(struct mm_struct *mm, unsigned long
ea,
#ifdef CONFIG_PPC32
+
+struct tlbcam {
+ u32 MAS0;
+ u32 MAS1;
+ u32 MAS2;
+ u32 MAS3;
+ u32 MAS7;
+};
+
extern void mapin_ram(void);
extern int map_page(unsigned long va, phys_addr_t pa, int flags);
extern void setbat(int index, unsigned long virt, phys_addr_t phys,
--
1.5.4.1
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH 2/5] powerpc: booke: Remove num_tlbcam_entries
This is a global variable defined in fsl_booke_mmu.c with a value that gets initialized in assembly code in head_fsl_booke.S. It's never used. If some code ever does want to know the number of entries in TLB1, then numcams = mfspr(SPRN_TLB1CFG) 0xfff, is a whole lot simpler than a global initialized during kernel boot from assembly. Signed-off-by: Trent Piepho [EMAIL PROTECTED] --- arch/powerpc/kernel/head_fsl_booke.S |4 arch/powerpc/mm/fsl_booke_mmu.c |1 - arch/powerpc/mm/mmu_decl.h |2 -- 3 files changed, 0 insertions(+), 7 deletions(-) diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S index c591acb..e33021f 100644 --- a/arch/powerpc/kernel/head_fsl_booke.S +++ b/arch/powerpc/kernel/head_fsl_booke.S @@ -389,10 +389,6 @@ skpinv:addir6,r6,1 /* Increment */ #endif #endif - mfspr r3,SPRN_TLB1CFG - andi. r3,r3,0xfff - lis r4,[EMAIL PROTECTED] - stw r3,[EMAIL PROTECTED](r4) /* * Decide what sort of machine this is and initialize the MMU. */ diff --git a/arch/powerpc/mm/fsl_booke_mmu.c b/arch/powerpc/mm/fsl_booke_mmu.c index c9ee59a..1971e4e 100644 --- a/arch/powerpc/mm/fsl_booke_mmu.c +++ b/arch/powerpc/mm/fsl_booke_mmu.c @@ -56,7 +56,6 @@ extern void loadcam_entry(unsigned int index); unsigned int tlbcam_index; -unsigned int num_tlbcam_entries; static unsigned long __cam0, __cam1, __cam2; #define NUM_TLBCAMS(16) diff --git a/arch/powerpc/mm/mmu_decl.h b/arch/powerpc/mm/mmu_decl.h index f0d0aae..1db3c67 100644 --- a/arch/powerpc/mm/mmu_decl.h +++ b/arch/powerpc/mm/mmu_decl.h @@ -51,8 +51,6 @@ extern unsigned int rtas_data, rtas_size; struct hash_pte; extern struct hash_pte *Hash, *Hash_end; extern unsigned long Hash_size, Hash_mask; - -extern unsigned int num_tlbcam_entries; #endif extern unsigned long ioremap_bot; -- 1.5.4.1 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH 3/5] powerpc: booke: Remove code duplication in lowmem mapping
The code to map lowmem uses three CAM aka TLB[1] entries to cover it. The
size of each is stored in three globals named __cam0, __cam1, and __cam2.
All the code that uses them is duplicated three times for each of the three
variables.
We have these things called arrays and loops
Once converted to use an array, it will be easier to make the number of
CAMs configurable.
Signed-off-by: Trent Piepho [EMAIL PROTECTED]
---
arch/powerpc/mm/fsl_booke_mmu.c | 79 +++---
1 files changed, 31 insertions(+), 48 deletions(-)
diff --git a/arch/powerpc/mm/fsl_booke_mmu.c b/arch/powerpc/mm/fsl_booke_mmu.c
index 1971e4e..1dabe1a 100644
--- a/arch/powerpc/mm/fsl_booke_mmu.c
+++ b/arch/powerpc/mm/fsl_booke_mmu.c
@@ -56,7 +56,7 @@
extern void loadcam_entry(unsigned int index);
unsigned int tlbcam_index;
-static unsigned long __cam0, __cam1, __cam2;
+static unsigned long cam[3];
#define NUM_TLBCAMS(16)
@@ -152,19 +152,19 @@ void invalidate_tlbcam_entry(int index)
loadcam_entry(index);
}
-void __init cam_mapin_ram(unsigned long cam0, unsigned long cam1,
- unsigned long cam2)
+unsigned long __init mmu_mapin_ram(void)
{
- settlbcam(0, PAGE_OFFSET, memstart_addr, cam0, _PAGE_KERNEL, 0);
- tlbcam_index++;
- if (cam1) {
- tlbcam_index++;
- settlbcam(1, PAGE_OFFSET+cam0, memstart_addr+cam0, cam1,
_PAGE_KERNEL, 0);
- }
- if (cam2) {
+ unsigned long virt = PAGE_OFFSET;
+ phys_addr_t phys = memstart_addr;
+
+ while (cam[tlbcam_index] tlbcam_index ARRAY_SIZE(cam)) {
+ settlbcam(tlbcam_index, virt, phys, cam[tlbcam_index],
_PAGE_KERNEL, 0);
+ virt += cam[tlbcam_index];
+ phys += cam[tlbcam_index];
tlbcam_index++;
- settlbcam(2, PAGE_OFFSET+cam0+cam1, memstart_addr+cam0+cam1,
cam2, _PAGE_KERNEL, 0);
}
+
+ return virt - PAGE_OFFSET;
}
/*
@@ -175,51 +175,34 @@ void __init MMU_init_hw(void)
flush_instruction_cache();
}
-unsigned long __init mmu_mapin_ram(void)
-{
- cam_mapin_ram(__cam0, __cam1, __cam2);
-
- return __cam0 + __cam1 + __cam2;
-}
-
-
void __init
adjust_total_lowmem(void)
{
- phys_addr_t max_lowmem_size = __max_low_memory;
- phys_addr_t cam_max_size = 0x1000;
phys_addr_t ram;
+ unsigned int max_cam = 28; /* 2^28 = 256 Mb */
+ char buf[ARRAY_SIZE(cam) * 5 + 1], *p = buf;
+ int i;
- /* adjust CAM size to max_lowmem_size */
- if (max_lowmem_size cam_max_size)
- cam_max_size = max_lowmem_size;
-
- /* adjust lowmem size to max_lowmem_size */
- ram = min(max_lowmem_size, total_lowmem);
+ /* adjust lowmem size to __max_low_memory */
+ ram = min((phys_addr_t)__max_low_memory, (phys_addr_t)total_lowmem);
/* Calculate CAM values */
- __cam0 = 1UL 2 * (__ilog2(ram) / 2);
- if (__cam0 cam_max_size)
- __cam0 = cam_max_size;
- ram -= __cam0;
- if (ram) {
- __cam1 = 1UL 2 * (__ilog2(ram) / 2);
- if (__cam1 cam_max_size)
- __cam1 = cam_max_size;
- ram -= __cam1;
- }
- if (ram) {
- __cam2 = 1UL 2 * (__ilog2(ram) / 2);
- if (__cam2 cam_max_size)
- __cam2 = cam_max_size;
- ram -= __cam2;
+ __max_low_memory = 0;
+ for (i = 0; ram i ARRAY_SIZE(cam); i++) {
+ unsigned int camsize = __ilog2(ram) ~1U;
+ if (camsize max_cam)
+ camsize = max_cam;
+ cam[i] = 1UL camsize;
+ ram -= cam[i];
+ __max_low_memory += cam[i];
+
+ p += sprintf(p, %lu/, cam[i] 20);
}
+ for (; i ARRAY_SIZE(cam); i++)
+ p += sprintf(p, 0/);
+ p[-1] = '\0';
- printk(KERN_INFO Memory CAM mapping: CAM0=%ldMb, CAM1=%ldMb,
-CAM2=%ldMb residual: %ldMb\n,
- __cam0 20, __cam1 20, __cam2 20,
- (long int)((total_lowmem - __cam0 - __cam1 - __cam2)
- 20));
- __max_low_memory = __cam0 + __cam1 + __cam2;
+ pr_info(Memory CAM mapping: %s Mb, residual: %ldMb\n, buf,
+ (total_lowmem - __max_low_memory) 20);
__initial_memory_limit_addr = memstart_addr + __max_low_memory;
}
--
1.5.4.1
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH 4/5] powerpc: booke: Make CAM entries used for lowmem configurable
On booke processors, the code that maps low memory only uses up to three
CAM entries, even though there are sixteen and nothing else uses them.
Make this number configurable in the advanced options menu along with max
low memory size. If one wants 1 GB of lowmem, then it's typically
necessary to have four CAM entries.
Signed-off-by: Trent Piepho [EMAIL PROTECTED]
---
arch/powerpc/Kconfig| 16
arch/powerpc/mm/fsl_booke_mmu.c |6 +-
2 files changed, 21 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index be4f99b..2bb645c 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -696,6 +696,22 @@ config LOWMEM_SIZE
hex Maximum low memory size (in bytes) if LOWMEM_SIZE_BOOL
default 0x3000
+config LOWMEM_CAM_NUM_BOOL
+ bool Set number of CAMs to use to map low memory
+ depends on ADVANCED_OPTIONS FSL_BOOKE
+ help
+ This option allows you to set the maximum number of CAM slots that
+ will be used to map low memory. There are a limited number of slots
+ available and even more limited number that will fit in the L1 MMU.
+ However, using more entries will allow mapping more low memory. This
+ can be useful in optimizing the layout of kernel virtual memory.
+
+ Say N here unless you know what you are doing.
+
+config LOWMEM_CAM_NUM
+ int Number of CAMs to use to map low memory if LOWMEM_CAM_NUM_BOOL
+ default 3
+
config RELOCATABLE
bool Build a relocatable kernel (EXPERIMENTAL)
depends on EXPERIMENTAL ADVANCED_OPTIONS FLATMEM FSL_BOOKE
diff --git a/arch/powerpc/mm/fsl_booke_mmu.c b/arch/powerpc/mm/fsl_booke_mmu.c
index 1dabe1a..73aa9b7 100644
--- a/arch/powerpc/mm/fsl_booke_mmu.c
+++ b/arch/powerpc/mm/fsl_booke_mmu.c
@@ -56,10 +56,14 @@
extern void loadcam_entry(unsigned int index);
unsigned int tlbcam_index;
-static unsigned long cam[3];
+static unsigned long cam[CONFIG_LOWMEM_CAM_NUM];
#define NUM_TLBCAMS(16)
+#if defined(CONFIG_LOWMEM_CAM_NUM_BOOL) (CONFIG_LOWMEM_CAM_NUM =
NUM_TLBCAMS)
+#error LOWMEM_CAM_NUM must be less than NUM_TLBCAMS
+#endif
+
struct tlbcam TLBCAM[NUM_TLBCAMS];
struct tlbcamrange {
--
1.5.4.1
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH 5/5] powerpc: booke: Allow larger CAM sizes than 256 MB
The code that maps kernel low memory would only use page sizes up to 256
MB. On E500v2 pages up to 4 GB are supported.
However, a page must be aligned to a multiple of the page's size. I.e.
256 MB pages must aligned to a 256 MB boundary. This was enforced by a
requirement that the physical and virtual addresses of the start of lowmem
be aligned to 256 MB. Clearly requiring 1GB or 4GB alignment to allow
pages of that size isn't acceptable.
To solve this, I simply have adjust_total_lowmem() take alignment into
account when it decides what size pages to use. Give it PAGE_OFFSET =
0x7000_, PHYSICAL_START = 0x3000_, and 2GB of RAM, and it will map
pages like this:
PA 0x3000_ VA 0x7000_ Size 256 MB
PA 0x4000_ VA 0x8000_ Size 1 GB
PA 0x8000_ VA 0xC000_ Size 256 MB
PA 0x9000_ VA 0xD000_ Size 256 MB
PA 0xA000_ VA 0xE000_ Size 256 MB
Because the lowmem mapping code now takes alignment into account,
PHYSICAL_ALIGN can be lowered from 256 MB to 64 MB. Even lower might be
possible. The lowmem code will work down to 4 kB but it's possible some of
the boot code will fail before then. Poor alignment will force small pages
to be used, which combined with the limited number of TLB1 pages available,
will result in very little memory getting mapped. So alignments less than
64 MB probably aren't very useful anyway.
Signed-off-by: Trent Piepho [EMAIL PROTECTED]
---
arch/powerpc/Kconfig|2 +-
arch/powerpc/mm/fsl_booke_mmu.c | 14 +-
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 2bb645c..a7b6b8f 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -776,7 +776,7 @@ config PHYSICAL_START
config PHYSICAL_ALIGN
hex
- default 0x1000 if FSL_BOOKE
+ default 0x0400 if FSL_BOOKE
help
This value puts the alignment restrictions on physical address
where kernel is loaded and run from. Kernel is compiled for an
diff --git a/arch/powerpc/mm/fsl_booke_mmu.c b/arch/powerpc/mm/fsl_booke_mmu.c
index 73aa9b7..0b9ba6b 100644
--- a/arch/powerpc/mm/fsl_booke_mmu.c
+++ b/arch/powerpc/mm/fsl_booke_mmu.c
@@ -183,9 +183,14 @@ void __init
adjust_total_lowmem(void)
{
phys_addr_t ram;
- unsigned int max_cam = 28; /* 2^28 = 256 Mb */
+ unsigned int max_cam = (mfspr(SPRN_TLB1CFG) 16) 0xff;
char buf[ARRAY_SIZE(cam) * 5 + 1], *p = buf;
int i;
+ unsigned long virt = PAGE_OFFSET 0xUL;
+ unsigned long phys = memstart_addr 0xUL;
+
+ /* Convert (4^max) kB to (2^max) bytes */
+ max_cam = max_cam * 2 + 10;
/* adjust lowmem size to __max_low_memory */
ram = min((phys_addr_t)__max_low_memory, (phys_addr_t)total_lowmem);
@@ -194,11 +199,18 @@ adjust_total_lowmem(void)
__max_low_memory = 0;
for (i = 0; ram i ARRAY_SIZE(cam); i++) {
unsigned int camsize = __ilog2(ram) ~1U;
+ unsigned int align = __ffs(virt | phys) ~1U;
+
+ if (camsize align)
+ camsize = align;
if (camsize max_cam)
camsize = max_cam;
+
cam[i] = 1UL camsize;
ram -= cam[i];
__max_low_memory += cam[i];
+ virt += cam[i];
+ phys += cam[i];
p += sprintf(p, %lu/, cam[i] 20);
}
--
1.5.4.1
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] POWERPC: Add cached region support to physmap_of MTD driver
The MTD system supports operation where a direct mapped flash chip is
mapped twice. The normal mapping is a standard ioremap(), which is
non-cached and guarded on powerpc. The second mapping is used only for
reads and can be cached and non-guarded. Currently, only the pxa2xx
mapping driver makes use of this feature. This patch adds support to the
physmap_of driver on PPC32 platforms for this cached mapping mode.
Because the flash chip doesn't participate in the cache coherency protocol,
it's necessary to invalidate the cache for parts of flash that are modified
with a program or erase operation. This is platform specific, for instance
the pxa2xx driver uses an ARM specific function. This patch adds
invalidate_dcache_icache_range() for PPC32 and uses it. Because of XIP,
it's entirely possible that the flash might be in the icache(*), so the
existing invalidate_dcache_range() function isn't enough.
Of course, a cached mapping can increase performance if the data is read
from cache instead of flash. But less obvious is that it can provide a
significant performance increase for cold-cache reads that still come from
flash. It allows efficient back-to-back reads and if the flash chip
controller support page burst mode, it allows that to be used as well.
The figures are for *cold-cache* read performance, measured on a Freescale
MPC8572 controlling a Spansion S29GL064N NOR flash chip. With and without
the flash being mapped cached and with and without the localbus controller
being programmed to use page burst mode:
Non-cached, w/o bursts: 13.61 MB/s
Non-cached, w/ bursts: 13.61 MB/s
Cached, w/o bursts: 16.75 MB/s 23% increase
Cached, w/ bursts: 44.79 MB/s 229% increase!
Even without any cache hits, the cached mapping provides a significant
increase in performance via improved bus utilization. Enabling burst
transfers is even more significant.
(*) The MTD device's -point() method, which is the mechanism for
supporting mmap and XIP, only allows for mmapping the uncached region. So
you can't actually XIP anything in the cache. But this could be fixed.
Signed-off-by: Trent Piepho [EMAIL PROTECTED]
---
Should this go in via powerpc tree or mtd?
arch/powerpc/include/asm/cacheflush.h |2 ++
arch/powerpc/kernel/misc_32.S | 21 +
drivers/mtd/maps/physmap_of.c | 20
3 files changed, 43 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/include/asm/cacheflush.h
b/arch/powerpc/include/asm/cacheflush.h
index ba667a3..385c26c 100644
--- a/arch/powerpc/include/asm/cacheflush.h
+++ b/arch/powerpc/include/asm/cacheflush.h
@@ -49,6 +49,8 @@ extern void flush_dcache_range(unsigned long start, unsigned
long stop);
#ifdef CONFIG_PPC32
extern void clean_dcache_range(unsigned long start, unsigned long stop);
extern void invalidate_dcache_range(unsigned long start, unsigned long stop);
+extern void invalidate_dcache_icache_range(unsigned long start,
+ unsigned long stop);
#endif /* CONFIG_PPC32 */
#ifdef CONFIG_PPC64
extern void flush_inval_dcache_range(unsigned long start, unsigned long stop);
diff --git a/arch/powerpc/kernel/misc_32.S b/arch/powerpc/kernel/misc_32.S
index bdc8b0e..ad12c9a 100644
--- a/arch/powerpc/kernel/misc_32.S
+++ b/arch/powerpc/kernel/misc_32.S
@@ -636,6 +636,27 @@ _GLOBAL(invalidate_dcache_range)
blr
/*
+ * Like above, but invalidate both the D-cache and I-cache. Used when a cached
+ * region has been modified from a source that does not participate in the
cache
+ * coherency protocol.
+ *
+ * invalidate_dcache_icache_range(unsigned long start, unsigned long stop)
+ */
+_GLOBAL(invalidate_dcache_icache_range)
+ clrrwi r3, r3, L1_CACHE_SHIFT /* start = ~((1SHIFT)-1) */
+ subfr4,r3,r4
+ addir4,r4,L1_CACHE_BYTES-1
+ srwi. r4,r4,L1_CACHE_SHIFT/* count = (start-stop+BYTES-1)/BYTES */
+ beqlr /* if (!count) return */
+ mtctr r4
+1: dcbi0,r3
+ icbi0,r3
+ addir3,r3,L1_CACHE_BYTES
+ bdnz1b
+ sync/* wait for [id]cbi's to get to ram */
+ blr
+
+/*
* Flush a particular page from the data cache to RAM.
* Note: this is necessary because the instruction cache does *not*
* snoop from the data cache.
diff --git a/drivers/mtd/maps/physmap_of.c b/drivers/mtd/maps/physmap_of.c
index 5fcfec0..e834298 100644
--- a/drivers/mtd/maps/physmap_of.c
+++ b/drivers/mtd/maps/physmap_of.c
@@ -32,6 +32,16 @@ struct of_flash {
#endif
};
+#ifdef CONFIG_PPC32
+#include asm/cacheflush.h
+static void ppc32_inval_cache(struct map_info *map, unsigned long from,
+ ssize_t len)
+{
+ invalidate_dcache_icache_range((unsigned long)map-cached + from,
+ (unsigned long)map-cached + from + len);
+}
+#endif
+
#ifdef CONFIG_MTD_PARTITIONS
#define OF_FLASH_PARTS
Re: [PATCH] powerpc: Fix bogus cache flushing on all 40x and BookE processors
On Tue, 2 Dec 2008, Benjamin Herrenschmidt wrote: On Tue, 2008-12-02 at 01:36 -0600, Kumar Gala wrote: #define CPU_FTRS_E200 (CPU_FTR_USE_TB | CPU_FTR_SPE_COMP | \ CPU_FTR_NODSISRALIGN | CPU_FTR_COHERENT_ICACHE | \ - CPU_FTR_UNIFIED_ID_CACHE) + CPU_FTR_UNIFIED_ID_CACHE | CPU_FTR_NOEXECUTE) #define CPU_FTRS_E500 (CPU_FTR_MAYBE_CAN_DOZE | CPU_FTR_USE_TB | \ - CPU_FTR_SPE_COMP | CPU_FTR_MAYBE_CAN_NAP | CPU_FTR_NODSISRALIGN) + CPU_FTR_SPE_COMP | CPU_FTR_MAYBE_CAN_NAP | CPU_FTR_NODSISRALIGN \ Added a '|' at the end of the line before the escape Right. Will send a new patch tomorrow. Appart from that, have you verified it doesn't have any adverse effects for you ? I did some quick tests on 440 and things seem to be fine. #ifdef __powerpc64__ #define LONG_ASM_CONST(x) ASM_CONST(x) #else #define LONG_ASM_CONST(x) 0 #endif #define CPU_FTR_NOEXECUTE LONG_ASM_CONST(0x0008) Am I not looking at the right code? Since e200 and e500 aren't powerpc64, doesn't adding CPU_FTR_NOEXECUTE have no effect at all? ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: __cpu_up vs. start_secondary race?
On Tue, 2 Dec 2008, Nathan Lynch wrote: Apart from barriers (or lack thereof), the fact that __cpu_up gives up after a more-or-less arbitrary period seems... well, arbitrary. If we get to Processor X is stuck then something is seriously wrong: there's either a kernel bug or a platform issue, and the CPU just kicked is in an unknown state. Polling indefinitely seems safer, no? I recently fixed a bug that did this. There was a bug in how the secondary CPU's memory was mapped (in some non-mailine code, not fixed). It was nice to get the warning and have the kernel not hang. On embedded systems with only network access and no persistent storage for system logs, a kernel hang is a lot more a pain. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [U-Boot] NAND only (no NOR)
On Wed, 3 Dec 2008, Sean MacLennan wrote: Yes, I would recommend to do it this way if possible. A small NOR for U-Boot and environment and everything else in NAND. This makes things much easier. But I understand that this is sometimes a problem with space (2 FLASH chips) and costs. Mainly cost. We didn't want to pay for a second chip. I think for NAND the latches necessary to de-multiplex the localbus aren't necessary like they are for NOR? On our board the latches might even take more space than the flash chip. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: i2c-mpc clocking scheme
On Mon, 1 Dec 2008, Andr? Schwarz wrote: Timur Tabi wrote: Trent Piepho wrote: Seems like it should keep the clock registers at what u-boot set them too. Or we could have U-Boot put the i2c clock frequency into the I2C node, and let the driver program the hardware again. That would keep the ugliness in U-Boot. Wouldn't it be easier to omit frequency re-programming at all ? Maybe configurable for non U-Boot users ... That's what I'm thinking too. Calculating the settings for a given bus frequency, even with the system specific source clock provided by u-boot, involves either a complex algorithm or a big ugly black box table that's even larger than the algorithm. I also think it's different for 8xxx and 52xx. On the other hand just keeping the clock the same doesn't require any code at all. U-boot could pass in bus-frequency to let software know the speed of the I2C bus from Linux. Seems like a standard property for bus nodes. There could be a current-speed property that tells linux to keep the registers the same, in case we have to worry about u-boot not programming the i2c clock. I think the serial drivers have something like this.___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: i2c-mpc clocking scheme
On Mon, 1 Dec 2008, Scott Wood wrote: Trent Piepho wrote: U-boot could pass in bus-frequency to let software know the speed of the I2C bus from Linux. Seems like a standard property for bus nodes. clock-frequency is standard, though it should probably be the input frequency rather than the bus frequency, in case the OS really does want to change it (maybe making the bus run faster when accessing faster devices). For a bus device like an i2c controller, you really have two clocks. The input clock the controller runs from and the speed it runs the bus at. One could say that one clock is for the device node and the other clock is for the device's sub-nodes. Linux doesn't have an API for setting I2C bus speed. If it did, then adding support for it to i2c-mpc if there was demand would really be another patch. Right now the problem is that Linux programs the I2C controller stupidly and undoes u-boot's (better) settings. There could be a current-speed property that tells linux to keep the registers the same, That would be a bit different from the way it's used in serial nodes, where current-speed is simply a description of the baud rate that corresponds to the current divider setting. I'm not sure that it makes as much sense for i2c, as you don't have the shared state on the other end that depends on maintaining the exact same speed. The Linux code could use current-speed to know if it should program the registers. I.e., if current-speed is present and non-zero, then leave the frequency registers alone. Otherwise u-boot or whatever might not have programmed the I2C controller and the driver can do what it's doing now. When does the guest really care what the specific i2c bus frequency is, if it's not going to change it? I don't know of a real reason. Maybe an I2C device where the clock speed makes a difference? Maximum polling rate or something? Is there reason the CPU clock and the CCB frequency need to be in the device tree? ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] of/gpio: Implement of_get_gpio_flags()
On Thu, 27 Nov 2008, Anton Vorontsov wrote:
This function is alike to the simple of_get_gpio(), but accepts new
argument: flags. This new function will be used by the drivers that
need to retrieve additional GPIO information, such as active-low flag.
Signed-off-by: Anton Vorontsov [EMAIL PROTECTED]
So you want to do the clean up patch later?
+ /*
+ * We're discouraging gpio_cells 2, since that way you'll have to
+ * write your own xlate function (that will have to retrive the GPIO
+ * number and the flags from a single gpio cell -- this is possible,
+ * but not recommended).
+ */
+ if (of_gc-gpio_cells 2) {
+ WARN_ON(1);
+ return -EINVAL;
+ }
If you're not going to allow 1 cell anymore (which should perhaps be
mentioned in the changelog), you could just check that when the of_gpio
chip is registered. There's no need to see if of_gc-gpio_cells has
changed each time a driver maps a GPIO.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: i2c-mpc clocking scheme
On Thu, 27 Nov 2008, Andre Schwarz wrote: Timur Tabi schrieb: On Thu, Nov 27, 2008 at 9:00 AM, Andre Schwarz [EMAIL PROTECTED] wrote: All, is anybody working on some improvements regarding configurable I2C frequency inside the i2c-mpc driver ? If not - would anybody be intersted in getting this done, i.e. configurable via device tree ? Maybe I'm missing something, but U-Boot configures the I2C bus speed. It does this because the algorithm is specific to the SOC itself. For example, the 8544 is different from the 8548. It would be a mess to duplicate this code in the kernel. You're right regarding U-Boot, but the i2c-mpc driver overwrites the frequency divider register on all chips. I'm not happy with a fixed 0x3f @ MPC5200 which results in 65kHz ... :-( Have a look at line 163 in drivers/i2c/busses/i2c-mpc.c Seems like it should keep the clock registers at what u-boot set them too. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2] of_gpio: Return GPIO flags from of_get_gpio()
On Thu, 27 Nov 2008, Paul Mackerras wrote: Anton Vorontsov writes: Can we apply it? Paul, Benjamin? The patchwork url for this patch is: http://patchwork.ozlabs.org/patch/6650/ Thanks! drivers/mtd/nand/fsl_upm.c |2 +- drivers/net/fs_enet/fs_enet-main.c |2 +- drivers/net/phy/mdio-ofgpio.c |4 ++-- drivers/of/gpio.c | 13 ++--- drivers/serial/cpm_uart/cpm_uart_core.c |2 +- include/linux/of_gpio.h | 21 + 6 files changed, 32 insertions(+), 12 deletions(-) That would need acks from Jeff Garzik and David Woodhouse. Alternatively you could add a new function (called, for instance, of_get_gpio_flags) with the extra parameter to eliminate the need to change any drivers at this stage, since they all seem to pass NULL for the flags argument. But if we did this every time any exported function needs to change, think how bloated the API would be with cruft. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: 8572E - machine check pin (MCP0)
On Mon, 24 Nov 2008, Morrison, Tom wrote: Running 2.6.23.25 kernel... I have an external watchdog timer that is going off - and pulsing into the MCP0 of the 8572E. I get the printk indicating that the MCP0 went off - the problem is - how do I clear the condition that caused this because my hardware engineer swears that the pulse is ONLY 250ms - and after resetting several status registers (mcpsumr rst (because my hardware engineer swears that the pulse is ONLY 250ms long (and I have a delay after my printk of 250ms)) - so I am pretty sure I am resetting the conditions mcpsumr (also, extra: the rstsr), but after writing mcpsumr - and reading back - it still has the mcp0 bit set? Where else do I need to reset the status - I think I am doing it right... but it isn't clearing the exception - and it 'dies' the next time through this (why is another problem - but first, I'd like to know why the condition is NOT being cleared...at all)... SRESET# also sets MCP0 and MCP1, maybe that is on? I'd also check the EMCP bit in SPRN_HID0 (on core 0 for MCP0). ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 4/4] leds: Let GPIO LEDs keep their current state
On Mon, 17 Nov 2008, Richard Purdie wrote:
On Fri, 2008-10-24 at 16:09 -0700, Trent Piepho wrote:
+if (template-keep_state)
+state = !!gpio_get_value(led_dat-gpio) ^ led_dat-active_low;
+else
+state = template-default_state;
state = of_get_property(child, default-state, NULL);
led.default_state = state !strcmp(state, on);
+led.keep_state = state !strcmp(state, keep);
+++ b/include/linux/leds.h
@@ -138,7 +138,8 @@ struct gpio_led {
const char *default_trigger;
unsignedgpio;
u8 active_low;
-u8 default_state;
+u8 default_state; /* 0 = off, 1 = on */
+u8 keep_state; /* overrides default_state */
};
How about something simpler here, just make default state have three
different values - keep, on and off? I'm not keen on having two
different state variables like this.
I thought of that, but it ends up being more complex. Instead of just
using:
static const struct gpio_led myled = {
.name = something,
.keep_state = 1,
}
You'd do something like this:
.default_state = LEDS_GPIO_DEFSTATE_KEEP,
Is that better? The constants for on/off/keep are one more thing you have
to look-up and remember when defining leds. The code in the leds-gpio
driver ends up getting more complex to deal with one tristate vs two
booleans.
This is a patch to change to a tristate. I don't think it's an
improvement. More symbols defined, more code, extra stuff to remember
when defining leds, and removing the field from struct gpio_led doesn't
make it smaller due to padding.
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index bb2a234..8a7303c 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -92,10 +92,10 @@ static int __devinit create_gpio_led(const struct gpio_led
*template,
led_dat-cdev.blink_set = gpio_blink_set;
}
led_dat-cdev.brightness_set = gpio_led_set;
- if (template-keep_state)
+ if (template-default_state == LEDS_GPIO_DEFSTATE_KEEP)
state = !!gpio_get_value(led_dat-gpio) ^ led_dat-active_low;
else
- state = template-default_state;
+ state = (template-default_state == LEDS_GPIO_DEFSTATE_ON);
led_dat-cdev.brightness = state ? LED_FULL : LED_OFF;
gpio_direction_output(led_dat-gpio, led_dat-active_low ^ state);
@@ -268,8 +268,15 @@ static int __devinit of_gpio_leds_probe(struct of_device
*ofdev,
led.default_trigger =
of_get_property(child, linux,default-trigger, NULL);
state = of_get_property(child, default-state, NULL);
- led.default_state = state !strcmp(state, on);
- led.keep_state = state !strcmp(state, keep);
+ if (state) {
+ if (!strcmp(state, keep)) {
+ led.default_state = LEDS_GPIO_DEFSTATE_KEEP;
+ } else if(!strcmp(state, on)) {
+ led.default_state = LEDS_GPIO_DEFSTATE_ON;
+ } else {
+ led.default_state = LEDS_GPIO_DEFSTATE_OFF;
+ }
+ }
ret = create_gpio_led(led, pdata-led_data[pdata-num_leds++],
ofdev-dev, NULL);
diff --git a/include/linux/leds.h b/include/linux/leds.h
index c51b625..f4a125c 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -138,9 +138,12 @@ struct gpio_led {
const char *default_trigger;
unsignedgpio;
u8 active_low;
- u8 default_state; /* 0 = off, 1 = on */
- u8 keep_state; /* overrides default_state */
+ u8 default_state;
+ /* default_state should be one of LEDS_GPIO_DEFSTATE_(ON|OFF|KEEP) */
};
+#define LEDS_GPIO_DEFSTATE_OFF 0
+#define LEDS_GPIO_DEFSTATE_ON 1
+#define LEDS_GPIO_DEFSTATE_KEEP2
struct gpio_led_platform_data {
int num_leds;
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] powerpc: L2 cache size wrong in 8572DS dts
It's 1MB, not 512KB. Newer U-Boots will fix this entry, but that's no reason to have the wrong value in the dts. Signed-off-by: Trent Piepho [EMAIL PROTECTED] Acked-by: Kumar Gala [EMAIL PROTECTED] --- arch/powerpc/boot/dts/mpc8572ds.dts |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/powerpc/boot/dts/mpc8572ds.dts b/arch/powerpc/boot/dts/mpc8572ds.dts index cadd465..5c69b2f 100644 --- a/arch/powerpc/boot/dts/mpc8572ds.dts +++ b/arch/powerpc/boot/dts/mpc8572ds.dts @@ -90,7 +90,7 @@ compatible = fsl,mpc8572-l2-cache-controller; reg = 0x2 0x1000; cache-line-size = 32; // 32 bytes - cache-size = 0x8; // L2, 512K + cache-size = 0x10; // L2, 1M interrupt-parent = mpic; interrupts = 16 2; }; -- 1.5.4.1 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] powerpc: Better setup of boot page TLB entry
The initial TLB mapping for the kernel boot didn't set the memory coherent attribute, MAS2[M], in SMP mode. If this code supported booting a secondary processor, which it doesn't yet, but suppose it did, then when a secondary processor boots, it would have probably signaled the primary processor by setting a variable called something like __secondary_hold_acknowledge. However, due to the lack of the M bit, the primary processor would not have snooped the transaction (even if a transaction were broadcast). If primary CPU's L1 D-cache had a copy, it would not have been flushed and the CPU would have never seen the ack. Which would have resulted in the primary CPU spinning for a long time, perhaps a full second before it would have given up, while it would have waited for the ack from the secondary CPU that it wouldn't have been able to see because of the stale cache. The value of MAS2 for the boot page TLB1 entry is a compile time constant, so there is no need to calculate it in powerpc assembly language. Also, from the MPC8572 manual section 6.12.5.3, Bits that represent offsets within a page are ignored and should be cleared. Existing code didn't clear them, this code does. The same when the page of KERNELBASE is found; we don't need to use asm to mask the lower 12 bits off. In the code that computes the address to rfi from, don't hard code the offset to 24 bytes, but have the assembler figure that out for us. Signed-off-by: Trent Piepho [EMAIL PROTECTED] Acked-by: Kumar Gala [EMAIL PROTECTED] --- arch/powerpc/include/asm/mmu-fsl-booke.h |2 ++ arch/powerpc/kernel/head_fsl_booke.S | 22 +- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/arch/powerpc/include/asm/mmu-fsl-booke.h b/arch/powerpc/include/asm/mmu-fsl-booke.h index 925d93c..5588a41 100644 --- a/arch/powerpc/include/asm/mmu-fsl-booke.h +++ b/arch/powerpc/include/asm/mmu-fsl-booke.h @@ -40,6 +40,8 @@ #define MAS2_M 0x0004 #define MAS2_G 0x0002 #define MAS2_E 0x0001 +#define MAS2_EPN_MASK(size)(~0 (2*(size) + 10)) +#define MAS2_VAL(addr, size, flags)((addr) MAS2_EPN_MASK(size) | (flags)) #define MAS3_RPN 0xF000 #define MAS3_U00x0200 diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S index 590304c..e621eac 100644 --- a/arch/powerpc/kernel/head_fsl_booke.S +++ b/arch/powerpc/kernel/head_fsl_booke.S @@ -235,36 +235,40 @@ skpinv: addir6,r6,1 /* Increment */ tlbivax 0,r9 TLBSYNC +/* The mapping only needs to be cache-coherent on SMP */ +#ifdef CONFIG_SMP +#defineM_IF_SMPMAS2_M +#else +#define M_IF_SMP 0 +#endif + /* 6. Setup KERNELBASE mapping in TLB1[0] */ lis r6,0x1000 /* Set MAS0(TLBSEL) = TLB1(1), ESEL = 0 */ mtspr SPRN_MAS0,r6 lis r6,(MAS1_VALID|MAS1_IPROT)@h ori r6,r6,(MAS1_TSIZE(BOOKE_PAGESZ_64M))@l mtspr SPRN_MAS1,r6 - li r7,0 - lis r6,[EMAIL PROTECTED] - ori r6,r6,[EMAIL PROTECTED] - rlwimi r6,r7,0,20,31 + lis r6,MAS2_VAL(PAGE_OFFSET, BOOKE_PAGESZ_64M, M_IF_SMP)@h + ori r6,r6,MAS2_VAL(PAGE_OFFSET, BOOKE_PAGESZ_64M, M_IF_SMP)@l mtspr SPRN_MAS2,r6 mtspr SPRN_MAS3,r8 tlbwe /* 7. Jump to KERNELBASE mapping */ - lis r6,[EMAIL PROTECTED] - ori r6,r6,[EMAIL PROTECTED] - rlwimi r6,r7,0,20,31 + lis r6,(KERNELBASE ~0xfff)@h + ori r6,r6,(KERNELBASE ~0xfff)@l lis r7,[EMAIL PROTECTED] ori r7,r7,[EMAIL PROTECTED] bl 1f /* Find our address */ 1: mflrr9 rlwimi r6,r9,0,20,31 - addir6,r6,24 + addir6,r6,(2f - 1b) mtspr SPRN_SRR0,r6 mtspr SPRN_SRR1,r7 rfi /* start execution out of TLB1[0] entry */ /* 8. Clear out the temp mapping */ - lis r7,0x1000 /* Set MAS0(TLBSEL) = 1 */ +2: lis r7,0x1000 /* Set MAS0(TLBSEL) = 1 */ rlwimi r7,r5,16,4,15 /* Setup MAS0 = TLBSEL | ESEL(r5) */ mtspr SPRN_MAS0,r7 tlbre -- 1.5.4.1 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Silence timebase sync code
On Tue, 18 Nov 2008, Michael Ellerman wrote:
On Mon, 2008-11-17 at 14:22 -0800, Trent Piepho wrote:
On Mon, 17 Nov 2008, Kumar Gala wrote:
On Nov 17, 2008, at 3:58 PM, Trent Piepho wrote:
It's over a dozen lines of output and doesn't appear to provide any useful
information. Even after looking at the code, I'm in the dark about what
score 299, offset 250 means.
Signed-off-by: Trent Piepho [EMAIL PROTECTED]
---
+++ b/arch/powerpc/kernel/smp-tbsync.c
@@ -113,7 +113,7 @@ void __devinit smp_generic_give_timebase(void)
{
int i, score, score2, old, min=0, max=5000, offset=1000;
- printk(Synchronizing timebase\n);
+ pr_info(Synchronizing timebase\n);
I think its useful to leave this as a printk.
#define pr_info(fmt, arg...) \
printk(KERN_INFO fmt, ##arg)
Isn't printk with no level tag the same as KERN_INFO?
Stuff like this should IMHO be printk(KERN_DEBUG ..)
That way it will show up in the log as long as you boot with 'debug' on
your command line, it doesn't require a kernel recompile to turn on. And
at the same time it doesn't spam the boot log for a normal boot.
Originally my patched changed it to debug, but Kumar requested I keep it at
info level. The timebase sync code might hang or be slow, so it's nice to
give a status output before doing it. It seems just as good as most of the
info printks during boot.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] powerpc: Silence timebase sync code
It's over a dozen lines of output and doesn't appear to provide any useful
information. Even after looking at the code, I'm in the dark about what
score 299, offset 250 means.
Signed-off-by: Trent Piepho [EMAIL PROTECTED]
---
arch/powerpc/kernel/smp-tbsync.c | 12
1 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/kernel/smp-tbsync.c b/arch/powerpc/kernel/smp-tbsync.c
index bc892e6..b590135 100644
--- a/arch/powerpc/kernel/smp-tbsync.c
+++ b/arch/powerpc/kernel/smp-tbsync.c
@@ -113,7 +113,7 @@ void __devinit smp_generic_give_timebase(void)
{
int i, score, score2, old, min=0, max=5000, offset=1000;
- printk(Synchronizing timebase\n);
+ pr_info(Synchronizing timebase\n);
/* if this fails then this kernel won't work anyway... */
tbsync = kzalloc( sizeof(*tbsync), GFP_KERNEL );
@@ -123,14 +123,10 @@ void __devinit smp_generic_give_timebase(void)
while (!tbsync-ack)
barrier();
- printk(Got ack\n);
-
/* binary search */
for (old = -1; old != offset ; offset = (min+max) / 2) {
score = start_contest(kSetAndTest, offset, NUM_ITER);
- printk(score %d, offset %d\n, score, offset );
-
if( score 0 )
max = offset;
else
@@ -140,8 +136,8 @@ void __devinit smp_generic_give_timebase(void)
score = start_contest(kSetAndTest, min, NUM_ITER);
score2 = start_contest(kSetAndTest, max, NUM_ITER);
- printk(Min %d (score %d), Max %d (score %d)\n,
- min, score, max, score2);
+ pr_debug(Min %d (score %d), Max %d (score %d)\n,
+min, score, max, score2);
score = abs(score);
score2 = abs(score2);
offset = (score score2) ? min : max;
@@ -155,7 +151,7 @@ void __devinit smp_generic_give_timebase(void)
if (score2 = score || score2 20)
break;
}
- printk(Final offset: %d (%d/%d)\n, offset, score2, NUM_ITER );
+ pr_debug(Final offset: %d (%d/%d)\n, offset, score2, NUM_ITER);
/* exiting */
tbsync-cmd = kExit;
--
1.5.4.1
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Silence timebase sync code
On Mon, 17 Nov 2008, Kumar Gala wrote:
On Nov 17, 2008, at 3:58 PM, Trent Piepho wrote:
It's over a dozen lines of output and doesn't appear to provide any useful
information. Even after looking at the code, I'm in the dark about what
score 299, offset 250 means.
Signed-off-by: Trent Piepho [EMAIL PROTECTED]
---
arch/powerpc/kernel/smp-tbsync.c | 12
1 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/kernel/smp-tbsync.c
b/arch/powerpc/kernel/smp-tbsync.c
index bc892e6..b590135 100644
--- a/arch/powerpc/kernel/smp-tbsync.c
+++ b/arch/powerpc/kernel/smp-tbsync.c
@@ -113,7 +113,7 @@ void __devinit smp_generic_give_timebase(void)
{
int i, score, score2, old, min=0, max=5000, offset=1000;
-printk(Synchronizing timebase\n);
+pr_info(Synchronizing timebase\n);
I think its useful to leave this as a printk.
#define pr_info(fmt, arg...) \
printk(KERN_INFO fmt, ##arg)
Isn't printk with no level tag the same as KERN_INFO?
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] powerpc: Repair device bindings documentation
Commit d0fc2eaaf4c56a95f5ed29b6bfb609e19714fc16 powerpc/fsl: Refactor
device bindings split out a number of device bindings from
booting-without-of.txt into separate files. Having them all in one file
was a frequent source of merge conflicts.
However, in the next merge, 49997d75152b3d23c53b0fa730599f2f74c92c65, there
was another conflict. Some of the bindings removed from
booting-without-of.txt were mistakenly added back in and the copies in
dts-bindings were kept as well.
This patch re-removes Freescale Display Interface and Freescale on board
FPGA and fixes the table of contents.
Signed-off-by: Trent Piepho [EMAIL PROTECTED]
Acked-by: Kumar Gala [EMAIL PROTECTED]
---
Maybe this should go in 2.6.28, since it fixes a doc bug and before
anyone modifies the version of the docs that should have been deleted.
Documentation/powerpc/booting-without-of.txt | 65 --
1 files changed, 10 insertions(+), 55 deletions(-)
diff --git a/Documentation/powerpc/booting-without-of.txt
b/Documentation/powerpc/booting-without-of.txt
index 02ea9a9..0ab0230 100644
--- a/Documentation/powerpc/booting-without-of.txt
+++ b/Documentation/powerpc/booting-without-of.txt
@@ -41,25 +41,14 @@ Table of Contents
VI - System-on-a-chip devices and nodes
1) Defining child nodes of an SOC
2) Representing devices without a current OF specification
- a) MDIO IO device
- b) Gianfar-compatible ethernet nodes
- c) PHY nodes
- d) Interrupt controllers
- e) I2C
- f) Freescale SOC USB controllers
- g) Freescale SOC SEC Security Engines
- h) Board Control and Status (BCSR)
- i) Freescale QUICC Engine module (QE)
- j) CFI or JEDEC memory-mapped NOR flash
- k) Global Utilities Block
- l) Freescale Communications Processor Module
- m) Chipselect/Local Bus
- n) 4xx/Axon EMAC ethernet nodes
- o) Xilinx IP cores
- p) Freescale Synchronous Serial Interface
- q) USB EHCI controllers
- r) MDIO on GPIOs
- s) SPI busses
+ a) PHY nodes
+ b) Interrupt controllers
+ c) CFI or JEDEC memory-mapped NOR flash
+ d) 4xx/Axon EMAC ethernet nodes
+ e) Xilinx IP cores
+ f) USB EHCI controllers
+ g) MDIO on GPIOs
+ h) SPI busses
VII - Marvell Discovery mv64[345]6x System Controller chips
1) The /system-controller node
@@ -1830,41 +1819,7 @@ platforms are moved over to use the
flattened-device-tree model.
big-endian;
};
-r) Freescale Display Interface Unit
-
-The Freescale DIU is a LCD controller, with proper hardware, it can also
-drive DVI monitors.
-
-Required properties:
-- compatible : should be fsl-diu.
-- reg : should contain at least address and length of the DIU register
- set.
-- Interrupts : one DIU interrupt should be describe here.
-
-Example (MPC8610HPCD)
- [EMAIL PROTECTED] {
- compatible = fsl,diu;
- reg = 0x2c000 100;
- interrupts = 72 2;
- interrupt-parent = mpic;
- };
-
-s) Freescale on board FPGA
-
-This is the memory-mapped registers for on board FPGA.
-
-Required properities:
-- compatible : should be fsl,fpga-pixis.
-- reg : should contain the address and the lenght of the FPPGA register
- set.
-
-Example (MPC8610HPCD)
- [EMAIL PROTECTED] {
- compatible = fsl,fpga-pixis;
- reg = 0xe800 32;
- };
-
- r) MDIO on GPIOs
+ g) MDIO on GPIOs
Currently defined compatibles:
- virtual,gpio-mdio
@@ -1884,7 +1839,7 @@ platforms are moved over to use the flattened-device-tree
model.
qe_pio_c 6;
};
-s) SPI (Serial Peripheral Interface) busses
+h) SPI (Serial Peripheral Interface) busses
SPI busses can be described with a node for the SPI master device
and a set of child nodes for each SPI slave on the bus. For this
--
1.5.4.1
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [v5] powerpc: gpio driver for mpc8349/8572/8610 and compatible
On Thu, 30 Oct 2008, Peter Korsgaard wrote: Trent == Trent Piepho [EMAIL PROTECTED] writes: Trent On Tue, 23 Sep 2008, Peter Korsgaard wrote: +- compatible : fsl,CHIP-gpio followed by fsl,mpc8349-gpio for + 83xx, fsl,mpc8572-gpio for 85xx and fsl,mpc8610-gpio for 86xx. Trent Why have the three different compatible settings when the code Trent doesn't do anything different? Purely for cosmetics / ease of use - As requested by Kumar: http://ozlabs.org/pipermail/linuxppc-dev/2008-September/062934.html Though I see Scott didn't agree. +#define MPC8XXX_GPIO_PINS 32 Trent 8572 has eight GPIOs. Doesn't matter - It's register interface is compatible. Most real world design with the other SoCs also don't have all 32 gpio pins available because of pin multiplexing. Doesn't it seem flawed that 32 gpios will show up under debugfs and via the sysfs interface, when it's known that there are only 8? Since there is an 8572 compat property, it's not like it's not solvable. Trent I wrote an MPC8572 GPIO driver back in March, and posted it Trent internally at Freescale on June 2nd. But it was just Trent ignored... I wonder what your secret is to get Kumar to apply Trent your patches? It's too bad this work keeps getting Trent duplicated. Did you try bribing? ;) No, seriously, why didn't you post it to linuxppc-dev, so the rest of the world would know about it? Well, I get totally ignored posting to an internal Freescale list, it doesn't seem like it will be any better posting to an external list. Trent But, I'm using the GPIOs to bit-bang a JTAG bus in the 20-30 Trent MHz range. The obvious GPIO driver is *much* too slow for Trent that. I got less than 3 MHz, and your driver looks like it Trent might be slightly slower than my initial driver. I would write a dedicated driver for something like that instead of using gpiolib. But the JTAG bus only uses four GPIOs, the other four get used for other things with gpiolib, like gpio leds for example. I also found it very handy to be able to see and modify all the gpio lines, even the ones the JTAG driver is using, via a sysfs interface to gpiolib. Gpiolib has quite some overhead compared to the actual work for changing a SoC gpio pin, but it also has some very nice Certainly is does, but there are ways to make it faster. Trent So I went to a lot of effort to speed it up and managed to Trent increase GPIO performance by nearly a factor of 10. Trying to Trent commit my driver at this point is probably hopeless and I Trent doubt anyone else cares about gpio speed. But at least the Trent number of gpios for 8572 can be fixed. Sure, going dedicated always can improve performance. I recently did some work in u-boot talking directly to the mpc83xx spi controller and got ~5x throughput compared to the Linux driver. It's not dedicated though. The gpiolib interface is still used. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] gianfar: Omit TBI auto-negotiation based on device tree
On Tue, 28 Oct 2008, Nate Case wrote: Some SGMII PHYs don't auto-negotiate correctly with the TBI+SerDes interface on the mpc85xx processors. Check for the sgmii-aneg-disable device tree flag and skip enabling auto-negotiation on the TBI side if present. Full duplex 1000 Mbit/s will be assumed for the SGMII link to the PHY (note that this does not affect the link speed on the external side of the external PHY). Note that there is a race in the tbi/serdes setup code. The writes to the TBI/SerDes with gfar_local_mdio_write() use the same MDIO bus registers as phylib uses to talk to the real phy or phys. There is no locking for gfar_local_mdio vs phylib so they can (and will) clobber each other. It doesn't usually happen, due to luck and general phylib slowness. But I've got some patches in 2.6.28 that speed up phylib and might makes this happen more often... But more relevant to your serdes problem, I also have a patch that prevents restarting serdes auto-negotiation if the serdes link is already up. My SGMII PHY will auto-negotiate, but it takes about 3 seconds. Avoiding an unnecessary 3 second auto-negotiation when the gianfar device is opened lets me cut my power-on to DHCP completion time in half. I wonder if this would also fix your problem, without needing to add the extra workaround? ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH 1/2] gianfar: Fix race in TBI/SerDes configuration
The init_phy() function attaches to the PHY, then configures the
SerDes-TBI link (in SGMII mode). The TBI is on the MDIO bus with the PHY
(sort of) and is accessed via the gianfar's MDIO registers, using the
functions gfar_local_mdio_read/write(), which don't do any locking.
The previously attached PHY will start a work-queue on a timer, and
probably an irq handler as well, which will talk to the PHY and thus use
the MDIO bus. This uses phy_read/write(), which have locking, but not
against the gfar_local_mdio versions.
The result is that PHY code will try to use the MDIO bus at the same time
as the SerDes setup code, corrupting the transfers.
Setting up the SerDes before attaching to the PHY will insure that there is
no race between the SerDes code and *our* PHY, but doesn't fix everything.
Typically the PHYs for all gianfar devices are on the same MDIO bus, which
is associated with the first gianfar device. This means that the first
gianfar's SerDes code could corrupt the MDIO transfers for a different
gianfar's PHY.
The lock used by phy_read/write() is contained in the mii_bus structure,
which is pointed to by the PHY. This is difficult to access from the
gianfar drivers, as there is no link between a gianfar device and the
mii_bus which shares the same MDIO registers. As far as the device layer
and drivers are concerned they are two unrelated devices (which happen to
share registers).
Generally all gianfar devices' PHYs will be on the bus associated with the
first gianfar. But this might not be the case, so simply locking the
gianfar's PHY's mii bus might not lock the mii bus that the SerDes setup
code is going to use.
We solve this by having the code that creates the gianfar platform device
look in the device tree for an mdio device that shares the gianfar's
registers. If one is found the ID of its platform device is saved in the
gianfar's platform data.
A new function in the gianfar mii code, gfar_get_miibus(), can use the bus
ID to search through the platform devices for a gianfar_mdio device with
the right ID. The platform device's driver data is the mii_bus structure,
which the SerDes setup code can use to lock the current bus.
Signed-off-by: Trent Piepho [EMAIL PROTECTED]
CC: Andy Fleming [EMAIL PROTECTED]
---
arch/powerpc/sysdev/fsl_soc.c | 26 ++
drivers/net/gianfar.c |7 +++
drivers/net/gianfar_mii.c | 21 +
drivers/net/gianfar_mii.h |3 +++
include/linux/fsl_devices.h |3 ++-
5 files changed, 59 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index 01b884b..26ecb96 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -223,6 +223,8 @@ static int gfar_mdio_of_init_one(struct device_node *np)
if (ret)
return ret;
+ /* The gianfar device will try to use the same ID created below to find
+* this bus, to coordinate register access (since they share). */
mdio_dev = platform_device_register_simple(fsl-gianfar_mdio,
res.start0xf, res, 1);
if (IS_ERR(mdio_dev))
@@ -394,6 +396,30 @@ static int __init gfar_of_init(void)
of_node_put(mdio);
}
+ /* Get MDIO bus controlled by this eTSEC, if any. Normally only
+* eTSEC 1 will control an MDIO bus, not necessarily the same
+* bus that its PHY is on ('mdio' above), so we can't just use
+* that. What we do is look for a gianfar mdio device that has
+* overlapping registers with this device. That's really the
+* whole point, to find the device sharing our registers to
+* coordinate access with it.
+*/
+ for_each_compatible_node(mdio, NULL, fsl,gianfar-mdio) {
+ if (of_address_to_resource(mdio, 0, res))
+ continue;
+
+ if (res.start = r[0].start res.end = r[0].end) {
+ /* Get the ID the mdio bus platform device was
+* registered with. gfar_data.bus_id is
+* different because it's for finding a PHY,
+* while this is for finding a MII bus.
+*/
+ gfar_data.mdio_bus = res.start0xf;
+ of_node_put(mdio);
+ break;
+ }
+ }
+
ret =
platform_device_add_data(gfar_dev, gfar_data,
sizeof(struct
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 64b2011..249541a 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -586,6 +586,10 @@ static void gfar_configure_serdes
[PATCH 2/2] gianfar: Don't reset TBI-SerDes link if it's already up
The link may be up already via the chip's reset strapping, or though action of U-Boot, or from the last time the interface was brought up. Resetting the link causes it to go down for several seconds. This can significantly increase the time from power-on to DHCP completion and a device being accessible to the network. Signed-off-by: Trent Piepho [EMAIL PROTECTED] Acked-by: Andy Fleming [EMAIL PROTECTED] --- drivers/net/gianfar.c |9 + 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c index 249541a..83a5cb6 100644 --- a/drivers/net/gianfar.c +++ b/drivers/net/gianfar.c @@ -591,6 +591,14 @@ static void gfar_configure_serdes(struct net_device *dev) if (bus) mutex_lock(bus-mdio_lock); + /* If the link is already up, we must already be ok, and don't need to +* configure and reset the TBI-SerDes link. Maybe U-Boot configured +* everything for us? Resetting it takes the link down and requires +* several seconds for it to come back. +*/ + if (gfar_local_mdio_read(regs, tbipa, MII_BMSR) BMSR_LSTATUS) + goto done; + /* Single clk mode, mii mode off(for serdes communication) */ gfar_local_mdio_write(regs, tbipa, MII_TBICON, TBICON_CLK_SELECT); @@ -601,6 +609,7 @@ static void gfar_configure_serdes(struct net_device *dev) gfar_local_mdio_write(regs, tbipa, MII_BMCR, BMCR_ANENABLE | BMCR_ANRESTART | BMCR_FULLDPLX | BMCR_SPEED1000); + done: if (bus) mutex_unlock(bus-mdio_lock); } -- 1.5.4.1 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH v2] of_gpio: Return GPIO flags from of_get_gpio()
The device binding spec for OF GPIOs defines a flags field, but there is
currently no way to get it. This patch adds a parameter to of_get_gpio()
where the flags will be returned if non-NULL. of_get_gpio() in turn passes
the parameter to the of_gpio_chip's xlate method, which can extract any
flags present from the gpio_spec.
The default (and currently only) of_gpio_chip xlate method,
of_gpio_simple_xlate(), is modified to do this.
Signed-off-by: Trent Piepho [EMAIL PROTECTED]
Acked-by: Grant Likely [EMAIL PROTECTED]
Acked-by: Anton Vorontsov [EMAIL PROTECTED]
---
Since this patch changes an API, it would be nice to get it into powerpc-next
soon so that people who have new patches that use this API, like Anton, can
base off it.
drivers/mtd/nand/fsl_upm.c |2 +-
drivers/net/fs_enet/fs_enet-main.c |2 +-
drivers/net/phy/mdio-ofgpio.c |4 ++--
drivers/of/gpio.c | 13 ++---
drivers/serial/cpm_uart/cpm_uart_core.c |2 +-
include/linux/of_gpio.h | 21 +
6 files changed, 32 insertions(+), 12 deletions(-)
diff --git a/drivers/mtd/nand/fsl_upm.c b/drivers/mtd/nand/fsl_upm.c
index 024e3ff..a25d962 100644
--- a/drivers/mtd/nand/fsl_upm.c
+++ b/drivers/mtd/nand/fsl_upm.c
@@ -218,7 +218,7 @@ static int __devinit fun_probe(struct of_device *ofdev,
}
fun-upm_cmd_offset = *prop;
- fun-rnb_gpio = of_get_gpio(ofdev-node, 0);
+ fun-rnb_gpio = of_get_gpio(ofdev-node, 0, NULL);
if (fun-rnb_gpio = 0) {
ret = gpio_request(fun-rnb_gpio, ofdev-dev.bus_id);
if (ret) {
diff --git a/drivers/net/fs_enet/fs_enet-main.c
b/drivers/net/fs_enet/fs_enet-main.c
index cb51c1f..5a3c7ee 100644
--- a/drivers/net/fs_enet/fs_enet-main.c
+++ b/drivers/net/fs_enet/fs_enet-main.c
@@ -994,7 +994,7 @@ static int __devinit find_phy(struct device_node *np,
goto out_put_phy;
}
- bus_id = of_get_gpio(mdionode, 0);
+ bus_id = of_get_gpio(mdionode, 0, NULL);
if (bus_id 0) {
struct resource res;
ret = of_address_to_resource(mdionode, 0, res);
diff --git a/drivers/net/phy/mdio-ofgpio.c b/drivers/net/phy/mdio-ofgpio.c
index 2ff9775..e3757c6 100644
--- a/drivers/net/phy/mdio-ofgpio.c
+++ b/drivers/net/phy/mdio-ofgpio.c
@@ -78,8 +78,8 @@ static int __devinit mdio_ofgpio_bitbang_init(struct mii_bus
*bus,
{
struct mdio_gpio_info *bitbang = bus-priv;
- bitbang-mdc = of_get_gpio(np, 0);
- bitbang-mdio = of_get_gpio(np, 1);
+ bitbang-mdc = of_get_gpio(np, 0, NULL);
+ bitbang-mdio = of_get_gpio(np, 1, NULL);
if (bitbang-mdc 0 || bitbang-mdio 0)
return -ENODEV;
diff --git a/drivers/of/gpio.c b/drivers/of/gpio.c
index 7cd7301..ae14215 100644
--- a/drivers/of/gpio.c
+++ b/drivers/of/gpio.c
@@ -22,11 +22,12 @@
* of_get_gpio - Get a GPIO number from the device tree to use with GPIO API
* @np:device node to get GPIO from
* @index: index of the GPIO
+ * @flags: GPIO's flags are returned here if non-NULL
*
* Returns GPIO number to use with Linux generic GPIO API, or one of the errno
* value on the error condition.
*/
-int of_get_gpio(struct device_node *np, int index)
+int of_get_gpio(struct device_node *np, int index, enum of_gpio_flags *flags)
{
int ret;
struct device_node *gc;
@@ -59,7 +60,9 @@ int of_get_gpio(struct device_node *np, int index)
goto err1;
}
- ret = of_gc-xlate(of_gc, np, gpio_spec);
+ if (flags)
+ *flags = 0;
+ ret = of_gc-xlate(of_gc, np, gpio_spec, flags);
if (ret 0)
goto err1;
@@ -77,19 +80,23 @@ EXPORT_SYMBOL(of_get_gpio);
* @of_gc: pointer to the of_gpio_chip structure
* @np:device node of the GPIO chip
* @gpio_spec: gpio specifier as found in the device tree
+ * @flags: if non-NULL flags are returned here
*
* This is simple translation function, suitable for the most 1:1 mapped
* gpio chips. This function performs only one sanity check: whether gpio
* is less than ngpios (that is specified in the gpio_chip).
*/
int of_gpio_simple_xlate(struct of_gpio_chip *of_gc, struct device_node *np,
-const void *gpio_spec)
+const void *gpio_spec, enum of_gpio_flags *flags)
{
const u32 *gpio = gpio_spec;
if (*gpio of_gc-gc.ngpio)
return -EINVAL;
+ if (flags of_gc-gpio_cells 1)
+ *flags = gpio[1];
+
return *gpio;
}
EXPORT_SYMBOL(of_gpio_simple_xlate);
diff --git a/drivers/serial/cpm_uart/cpm_uart_core.c
b/drivers/serial/cpm_uart/cpm_uart_core.c
index bde4b4b..7835cd4 100644
--- a/drivers/serial/cpm_uart/cpm_uart_core.c
+++ b/drivers/serial/cpm_uart/cpm_uart_core.c
@@ -1094,7 +1094,7 @@ static int cpm_uart_init_port(struct device_node *np
Re: [v5] powerpc: gpio driver for mpc8349/8572/8610 and compatible
On Tue, 23 Sep 2008, Peter Korsgaard wrote: +- compatible : fsl,CHIP-gpio followed by fsl,mpc8349-gpio for + 83xx, fsl,mpc8572-gpio for 85xx and fsl,mpc8610-gpio for 86xx. Why have the three different compatible settings when the code doesn't do anything different? +#define MPC8XXX_GPIO_PINS32 8572 has eight GPIOs. I wrote an MPC8572 GPIO driver back in March, and posted it internally at Freescale on June 2nd. But it was just ignored... I wonder what your secret is to get Kumar to apply your patches? It's too bad this work keeps getting duplicated. My patch started out *very* much like yours, except it pre-dated the OF gpio controller stuff and of_mm_gpiochip so it didn't use that. But, I'm using the GPIOs to bit-bang a JTAG bus in the 20-30 MHz range. The obvious GPIO driver is *much* too slow for that. I got less than 3 MHz, and your driver looks like it might be slightly slower than my initial driver. So I went to a lot of effort to speed it up and managed to increase GPIO performance by nearly a factor of 10. Trying to commit my driver at this point is probably hopeless and I doubt anyone else cares about gpio speed. But at least the number of gpios for 8572 can be fixed. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 1/4] of_gpio: Return GPIO flags from of_get_gpio()A
On Tue, 28 Oct 2008, Anton Vorontsov wrote: On Fri, Oct 24, 2008 at 04:08:58PM -0700, Trent Piepho wrote: + * @flags: if non-NUll flags are returned here NULL, not NUll. Thanks, fixed. + const void *gpio_spec, unsigned int *flags) Why you made it unsigned int? In my original patch, I used named enum, which is self-documenting type. I started writing this patch before you posted yours, and I didn't think of the enum. But you have a good point so I'll switch to an enum. + * Flags as returned by OF GPIO chip's xlate function. + * These do not need to be the same as the flags in the GPIO specifier in the + * OF device tree, but it's convenient if they are. The mm chip OF GPIO + * driver works this way. This is not of_mm_gpio_chip specific. of_mm_gpio_chip was just an example of a driver that uses the same flag format for Linux and the OF binding. I'll clarify the comment. WRT changing the interface, Linux doesn't provide a stable kernel API. Functions that have been around far longer than of_get_gpio() and have far more users have changed. Yes, it is slightly annoying now. But providing backward compatibility for every single interface change will produce a bloated and redundant API that will be around forever. Can you repost a fixed version with my Ack and Cc: Andrew Morton, Benjamin Herrenschmidt? I think this change should go into the 2.6.28, so that we can write new code on top of new API. Otherwise this change will cause issues in the next merge window. If you can get your patch into Ben's -next tree before the high .28-RCs come out, I can just rebase my patch to that tree and make the changes to any new callers of of_get_gpio() that are there. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
OpenFirmware GPIO LED driver
This series of patches adds support for OpenFirmware bindings for GPIO based LEDs. I last posted a version of this in July but discussion of the OF binding details didn't seem to be going anywhere. I've since been contacted by some people who are actually using the previous patches and have been motivated to try again. All the users of this code are satisfied with the current version and it does everything they need it to do. The first patch extends the of_get_gpio() function to provide the gpio flags. The way this already works (i.e., this is not something I'm adding, it's what's already there) is that the OF gpio specifier is an opaque sequence of words. The GPIO controller driver (of which only one currently exists) provides an -xlate() method that turns this into a Linux gpiolib gpio number. All current gpio controllers have flags in their gpio specifier, but there is no way to get them. So I extend the xlate method to also produce the flags in a Linux format, the same way it produces a Linux gpio number. The second patch adds OF bindings to the gpio-leds driver. The existing code is refactored so that almost all of the common code is shared between the two binding methods. Either or both bindings can be selected via Kconfig. The bindings do have one linux, property, but no one has been able to come up with something close to workable that avoids this and still satisfies the *requirement* that the default trigger be settable from the OF bindings. The second and third patches add some additional capabilities for gpio leds that some users need. One is to have a LED start in the on state when it's made known to Linux. There is already a default-on trigger that does this, but it produces a glitch where an LED that is already on will turn off then back on. My (tiny) patch avoids this problem. The next lets an LED stay, without glitches, in whatever state it was already in when it's made known to Linux. It may have been put into this state by the BIOS, firmware, bootloader, or the hardware itself. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH 1/4] of_gpio: Return GPIO flags from of_get_gpio()
The device binding spec for OF GPIOs defines a flags field, but there is
currently no way to get it. This patch adds a parameter to of_get_gpio()
where the flags will be returned if non-NULL. of_get_gpio() in turn passes
the parameter to the of_gpio_chip's xlate method, which can extract any
flags present from the gpio_spec.
The default (and currently only) of_gpio_chip xlate method,
of_gpio_simple_xlate(), is modified to do this.
Signed-off-by: Trent Piepho [EMAIL PROTECTED]
---
drivers/mtd/nand/fsl_upm.c |2 +-
drivers/net/fs_enet/fs_enet-main.c |2 +-
drivers/net/phy/mdio-ofgpio.c |4 ++--
drivers/of/gpio.c | 13 ++---
drivers/serial/cpm_uart/cpm_uart_core.c |2 +-
include/linux/of_gpio.h | 17 +
6 files changed, 28 insertions(+), 12 deletions(-)
diff --git a/drivers/mtd/nand/fsl_upm.c b/drivers/mtd/nand/fsl_upm.c
index 024e3ff..a25d962 100644
--- a/drivers/mtd/nand/fsl_upm.c
+++ b/drivers/mtd/nand/fsl_upm.c
@@ -218,7 +218,7 @@ static int __devinit fun_probe(struct of_device *ofdev,
}
fun-upm_cmd_offset = *prop;
- fun-rnb_gpio = of_get_gpio(ofdev-node, 0);
+ fun-rnb_gpio = of_get_gpio(ofdev-node, 0, NULL);
if (fun-rnb_gpio = 0) {
ret = gpio_request(fun-rnb_gpio, ofdev-dev.bus_id);
if (ret) {
diff --git a/drivers/net/fs_enet/fs_enet-main.c
b/drivers/net/fs_enet/fs_enet-main.c
index cb51c1f..5a3c7ee 100644
--- a/drivers/net/fs_enet/fs_enet-main.c
+++ b/drivers/net/fs_enet/fs_enet-main.c
@@ -994,7 +994,7 @@ static int __devinit find_phy(struct device_node *np,
goto out_put_phy;
}
- bus_id = of_get_gpio(mdionode, 0);
+ bus_id = of_get_gpio(mdionode, 0, NULL);
if (bus_id 0) {
struct resource res;
ret = of_address_to_resource(mdionode, 0, res);
diff --git a/drivers/net/phy/mdio-ofgpio.c b/drivers/net/phy/mdio-ofgpio.c
index 2ff9775..e3757c6 100644
--- a/drivers/net/phy/mdio-ofgpio.c
+++ b/drivers/net/phy/mdio-ofgpio.c
@@ -78,8 +78,8 @@ static int __devinit mdio_ofgpio_bitbang_init(struct mii_bus
*bus,
{
struct mdio_gpio_info *bitbang = bus-priv;
- bitbang-mdc = of_get_gpio(np, 0);
- bitbang-mdio = of_get_gpio(np, 1);
+ bitbang-mdc = of_get_gpio(np, 0, NULL);
+ bitbang-mdio = of_get_gpio(np, 1, NULL);
if (bitbang-mdc 0 || bitbang-mdio 0)
return -ENODEV;
diff --git a/drivers/of/gpio.c b/drivers/of/gpio.c
index 7cd7301..2123517 100644
--- a/drivers/of/gpio.c
+++ b/drivers/of/gpio.c
@@ -22,11 +22,12 @@
* of_get_gpio - Get a GPIO number from the device tree to use with GPIO API
* @np:device node to get GPIO from
* @index: index of the GPIO
+ * @flags: GPIO's flags are returned here if non-NULL
*
* Returns GPIO number to use with Linux generic GPIO API, or one of the errno
* value on the error condition.
*/
-int of_get_gpio(struct device_node *np, int index)
+int of_get_gpio(struct device_node *np, int index, unsigned int *flags)
{
int ret;
struct device_node *gc;
@@ -59,7 +60,9 @@ int of_get_gpio(struct device_node *np, int index)
goto err1;
}
- ret = of_gc-xlate(of_gc, np, gpio_spec);
+ if (flags)
+ *flags = 0;
+ ret = of_gc-xlate(of_gc, np, gpio_spec, flags);
if (ret 0)
goto err1;
@@ -77,19 +80,23 @@ EXPORT_SYMBOL(of_get_gpio);
* @of_gc: pointer to the of_gpio_chip structure
* @np:device node of the GPIO chip
* @gpio_spec: gpio specifier as found in the device tree
+ * @flags: if non-NUll flags are returned here
*
* This is simple translation function, suitable for the most 1:1 mapped
* gpio chips. This function performs only one sanity check: whether gpio
* is less than ngpios (that is specified in the gpio_chip).
*/
int of_gpio_simple_xlate(struct of_gpio_chip *of_gc, struct device_node *np,
-const void *gpio_spec)
+const void *gpio_spec, unsigned int *flags)
{
const u32 *gpio = gpio_spec;
if (*gpio of_gc-gc.ngpio)
return -EINVAL;
+ if (flags of_gc-gpio_cells 1)
+ *flags = gpio[1];
+
return *gpio;
}
EXPORT_SYMBOL(of_gpio_simple_xlate);
diff --git a/drivers/serial/cpm_uart/cpm_uart_core.c
b/drivers/serial/cpm_uart/cpm_uart_core.c
index bde4b4b..7835cd4 100644
--- a/drivers/serial/cpm_uart/cpm_uart_core.c
+++ b/drivers/serial/cpm_uart/cpm_uart_core.c
@@ -1094,7 +1094,7 @@ static int cpm_uart_init_port(struct device_node *np,
}
for (i = 0; i NUM_GPIOS; i++)
- pinfo-gpios[i] = of_get_gpio(np, i);
+ pinfo-gpios[i] = of_get_gpio(np, i, NULL);
return cpm_uart_request_port(pinfo-port);
diff --git a/include/linux/of_gpio.h b/include/linux
[PATCH 2/4] leds: Support OpenFirmware led bindings
Add bindings to support LEDs defined as of_platform devices in addition to
the existing bindings for platform devices.
New options in Kconfig allow the platform binding code and/or the
of_platform code to be turned on. The of_platform code is of course only
available on archs that have OF support.
The existing probe and remove methods are refactored to use new functions
create_gpio_led(), to create and register one led, and delete_gpio_led(),
to unregister and free one led. The new probe and remove methods for the
of_platform driver can then share most of the common probe and remove code
with the platform driver.
The suspend and resume methods aren't shared, but they are very short. The
actual led driving code is the same for LEDs created by either binding.
The OF bindings are based on patch by Anton Vorontsov
[EMAIL PROTECTED]. They have been extended to allow multiple LEDs
per device.
Signed-off-by: Trent Piepho [EMAIL PROTECTED]
---
Documentation/powerpc/dts-bindings/gpio/led.txt | 46 -
drivers/leds/Kconfig| 21 ++-
drivers/leds/leds-gpio.c| 230 ++-
3 files changed, 243 insertions(+), 54 deletions(-)
diff --git a/Documentation/powerpc/dts-bindings/gpio/led.txt
b/Documentation/powerpc/dts-bindings/gpio/led.txt
index ff51f4c..9f969c2 100644
--- a/Documentation/powerpc/dts-bindings/gpio/led.txt
+++ b/Documentation/powerpc/dts-bindings/gpio/led.txt
@@ -1,15 +1,43 @@
-LED connected to GPIO
+LEDs connected to GPIO lines
Required properties:
-- compatible : should be gpio-led.
-- label : (optional) the label for this LED. If omitted, the label is
+- compatible : should be gpio-leds.
+
+Each LED is represented as a sub-node of the gpio-leds device. Each
+node's name represents the name of the corresponding LED.
+
+LED sub-node properties:
+- gpios : Should specify the LED's GPIO, see Specifying GPIO information
+ for devices in Documentation/powerpc/booting-without-of.txt. Active
+ low LEDs should be indicated using flags in the GPIO specifier.
+- label : (optional) The label for this LED. If omitted, the label is
taken from the node name (excluding the unit address).
-- gpios : should specify LED GPIO.
+- linux,default-trigger : (optional) This parameter, if present, is a
+ string defining the trigger assigned to the LED. Current triggers are:
+backlight - LED will act as a back-light, controlled by the framebuffer
+ system
+default-on - LED will turn on
+heartbeat - LED double flashes at a load average based rate
+ide-disk - LED indicates disk activity
+timer - LED flashes at a fixed, configurable rate
-Example:
+Examples:
[EMAIL PROTECTED] {
- compatible = gpio-led;
- label = hdd;
- gpios = mcu_pio 0 1;
+leds {
+ compatible = gpio-leds;
+ hdd {
+ label = IDE Activity;
+ gpios = mcu_pio 0 1; /* Active low */
+ linux,default-trigger = ide-disk;
+ };
};
+
+run-control {
+ compatible = gpio-leds;
+ red {
+ gpios = mpc8572 6 0;
+ };
+ green {
+ gpios = mpc8572 7 0;
+ };
+}
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index e7fb7d2..6c6dc96 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -111,7 +111,26 @@ config LEDS_GPIO
help
This option enables support for the LEDs connected to GPIO
outputs. To be useful the particular board must have LEDs
- and they must be connected to the GPIO lines.
+ and they must be connected to the GPIO lines. The LEDs must be
+ defined as platform devices and/or OpenFirmware platform devices.
+ The code to use these bindings can be selected below.
+
+config LEDS_GPIO_PLATFORM
+ bool Platform device bindings for GPIO LEDs
+ depends on LEDS_GPIO
+ default y
+ help
+ Let the leds-gpio driver drive LEDs which have been defined as
+ platform devices. If you don't know what this means, say yes.
+
+config LEDS_GPIO_OF
+ bool OpenFirmware platform device bindings for GPIO LEDs
+ depends on LEDS_GPIO OF_DEVICE
+ default y
+ help
+ Let the leds-gpio driver drive LEDs which have been defined as
+ of_platform devices. For instance, LEDs which are listed in a dts
+ file.
config LEDS_HP_DISK
tristate LED Support for disk protection LED on HP notebooks
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index b13bd29..f41b841 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -3,6 +3,7 @@
*
* Copyright (C) 2007 8D Technologies inc.
* Raphael Assenat [EMAIL PROTECTED]
+ * Copyright (C) 2008 Freescale Semiconductor, Inc.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
@@ -71,50 +72,67 @@ static
[PATCH 3/4] leds: Add option to have GPIO LEDs start on
Yes, there is the default-on trigger but there are problems with that.
For one, it's a inefficient way to do it and requires led trigger support
to be compiled in.
But the real reason is that is produces a glitch on the LED. The GPIO is
allocate with the LED *off*, then *later* when then trigger runs it is
turned back on. If the LED was already on via the GPIO's reset default or
action of the firmware, this produces a glitch where the LED goes from on
to off to on. While normally this is fast enough that it wouldn't be
noticeable to a human observer, there are still serious problems.
One is that there may be something else on the GPIO line, like a hardware
alarm or watchdog, that is fast enough to notice the glitch.
Another is that the kernel may panic before the LED is turned back on, thus
hanging with the LED in the wrong state. This is not just speculation, but
actually happened to me with an embedded system that has an LED which
should turn off when the kernel finishes booting, which was left in the
incorrect state due to a bug in the OF LED binding code.
The platform device binding gains a field in the platform data default_state
that controls this. The OpenFirmware binding uses a property named
default-state that can be set to on or off. The default the property
isn't present is off.
Signed-off-by: Trent Piepho [EMAIL PROTECTED]
---
Documentation/powerpc/dts-bindings/gpio/led.txt |7 +++
drivers/leds/leds-gpio.c|8 ++--
include/linux/leds.h|1 +
3 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/Documentation/powerpc/dts-bindings/gpio/led.txt
b/Documentation/powerpc/dts-bindings/gpio/led.txt
index 9f969c2..544ded7 100644
--- a/Documentation/powerpc/dts-bindings/gpio/led.txt
+++ b/Documentation/powerpc/dts-bindings/gpio/led.txt
@@ -20,6 +20,11 @@ LED sub-node properties:
heartbeat - LED double flashes at a load average based rate
ide-disk - LED indicates disk activity
timer - LED flashes at a fixed, configurable rate
+- default-state: (optional) The initial state of the LED. Valid
+ values are on and off. If the LED is already on or off and the
+ default-state property is set the to same value, then no glitch
+ should be produced where the LED momentarily turns off (or on).
+ The default is off if this property is not present.
Examples:
@@ -36,8 +41,10 @@ run-control {
compatible = gpio-leds;
red {
gpios = mpc8572 6 0;
+ default-state = off;
};
green {
gpios = mpc8572 7 0;
+ default-state = on;
};
}
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index f41b841..0dbad87 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -92,9 +92,10 @@ static int __devinit create_gpio_led(const struct gpio_led
*template,
led_dat-cdev.blink_set = gpio_blink_set;
}
led_dat-cdev.brightness_set = gpio_led_set;
- led_dat-cdev.brightness = LED_OFF;
+ led_dat-cdev.brightness = template-default_state ? LED_FULL : LED_OFF;
- gpio_direction_output(led_dat-gpio, led_dat-active_low);
+ gpio_direction_output(led_dat-gpio,
+ led_dat-active_low ^ template-default_state);
INIT_WORK(led_dat-work, gpio_led_work);
@@ -256,12 +257,15 @@ static int __devinit of_gpio_leds_probe(struct of_device
*ofdev,
memset(led, 0, sizeof(led));
for_each_child_of_node(np, child) {
unsigned int flags;
+ const char *state;
led.gpio = of_get_gpio(child, 0, flags);
led.active_low = flags OF_GPIO_ACTIVE_LOW;
led.name = of_get_property(child, label, NULL) ? :
child-name;
led.default_trigger =
of_get_property(child, linux,default-trigger, NULL);
+ state = of_get_property(child, default-state, NULL);
+ led.default_state = state !strcmp(state, on);
ret = create_gpio_led(led, pdata-led_data[pdata-num_leds++],
ofdev-dev, NULL);
diff --git a/include/linux/leds.h b/include/linux/leds.h
index d3a73f5..caa3987 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -138,6 +138,7 @@ struct gpio_led {
const char *default_trigger;
unsignedgpio;
u8 active_low;
+ u8 default_state;
};
struct gpio_led_platform_data {
--
1.5.4.3
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH 4/4] leds: Let GPIO LEDs keep their current state
Let GPIO LEDs get their initial value from whatever the current state of
the GPIO line is. On some systems the LEDs are put into some state by the
firmware or hardware before Linux boots, and it is desired to have them
keep this state which is otherwise unknown to Linux.
This requires that the underlying GPIO driver support reading the value of
output GPIOs. Some drivers support this and some do not.
The platform data for the platform device binding gets a new field
'keep_state' which turns this on. keep_state overrides default_state.
For the OpenFirmware bindings, the default-state property gains a new
allowable setting keep.
Signed-off-by: Trent Piepho [EMAIL PROTECTED]
---
Documentation/powerpc/dts-bindings/gpio/led.txt | 16
drivers/leds/leds-gpio.c| 12
include/linux/leds.h|3 ++-
3 files changed, 22 insertions(+), 9 deletions(-)
diff --git a/Documentation/powerpc/dts-bindings/gpio/led.txt
b/Documentation/powerpc/dts-bindings/gpio/led.txt
index 544ded7..918bf9f 100644
--- a/Documentation/powerpc/dts-bindings/gpio/led.txt
+++ b/Documentation/powerpc/dts-bindings/gpio/led.txt
@@ -21,10 +21,12 @@ LED sub-node properties:
ide-disk - LED indicates disk activity
timer - LED flashes at a fixed, configurable rate
- default-state: (optional) The initial state of the LED. Valid
- values are on and off. If the LED is already on or off and the
- default-state property is set the to same value, then no glitch
- should be produced where the LED momentarily turns off (or on).
- The default is off if this property is not present.
+ values are on, off, and keep. If the LED is already on or off
+ and the default-state property is set the to same value, then no
+ glitch should be produced where the LED momentarily turns off (or
+ on). The keep setting will keep the LED at whatever its current
+ state is, without producing a glitch. The default is off if this
+ property is not present.
Examples:
@@ -35,6 +37,12 @@ leds {
gpios = mcu_pio 0 1; /* Active low */
linux,default-trigger = ide-disk;
};
+
+ fault {
+ gpios = mcu_pio 1 0;
+ /* Keep LED on if BIOS detected hardware fault */
+ default-state = keep;
+ };
};
run-control {
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index 0dbad87..bb2a234 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -76,7 +76,7 @@ static int __devinit create_gpio_led(const struct gpio_led
*template,
struct gpio_led_data *led_dat, struct device *parent,
int (*blink_set)(unsigned, unsigned long *, unsigned long *))
{
- int ret;
+ int ret, state;
ret = gpio_request(template-gpio, template-name);
if (ret 0)
@@ -92,10 +92,13 @@ static int __devinit create_gpio_led(const struct gpio_led
*template,
led_dat-cdev.blink_set = gpio_blink_set;
}
led_dat-cdev.brightness_set = gpio_led_set;
- led_dat-cdev.brightness = template-default_state ? LED_FULL : LED_OFF;
+ if (template-keep_state)
+ state = !!gpio_get_value(led_dat-gpio) ^ led_dat-active_low;
+ else
+ state = template-default_state;
+ led_dat-cdev.brightness = state ? LED_FULL : LED_OFF;
- gpio_direction_output(led_dat-gpio,
- led_dat-active_low ^ template-default_state);
+ gpio_direction_output(led_dat-gpio, led_dat-active_low ^ state);
INIT_WORK(led_dat-work, gpio_led_work);
@@ -266,6 +269,7 @@ static int __devinit of_gpio_leds_probe(struct of_device
*ofdev,
of_get_property(child, linux,default-trigger, NULL);
state = of_get_property(child, default-state, NULL);
led.default_state = state !strcmp(state, on);
+ led.keep_state = state !strcmp(state, keep);
ret = create_gpio_led(led, pdata-led_data[pdata-num_leds++],
ofdev-dev, NULL);
diff --git a/include/linux/leds.h b/include/linux/leds.h
index caa3987..c51b625 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -138,7 +138,8 @@ struct gpio_led {
const char *default_trigger;
unsignedgpio;
u8 active_low;
- u8 default_state;
+ u8 default_state; /* 0 = off, 1 = on */
+ u8 keep_state; /* overrides default_state */
};
struct gpio_led_platform_data {
--
1.5.4.3
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] powerpc/fsl: Refactor device bindings
On Wed, 9 Jul 2008, Kumar Gala wrote: Moved Freescale SoC related bindings out of booting-without-of.txt and into their own files. Signed-off-by: Kumar Gala galak at kernel.crashing.org --- We need to resolve some conflicts in cpm.txt and qe.txt but that will be a followup patch. - k Documentation/powerpc/booting-without-of.txt | 1218 +--- It looks like the part of the this patch that deleted all the bindings from booting-without-of.txt was paritially reverted in a subsequent merge. The table of contents still lists all the sections that were removed and sections r) and s) were added back in. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 1/2] leds: make the default trigger name const
The default_trigger fields of struct gpio_led and thus struct led_classdev
are pretty much always assigned from a string literal, which means the
string can't be modified. Which is fine, since there is no reason to
modify the string and in fact it never is.
But they should be marked const to prevent such code from being added, to
prevent warnings if -Wwrite-strings is used and when assigned from a
constant string other than a string literal (which produces a warning under
current kernel compiler flags), and for general good coding practices.
Signed-off-by: Trent Piepho [EMAIL PROTECTED]
---
Reposting this again. It still applies to powerpc-next as of Aug 28 and I
don't think anyone had any problems with it.
include/linux/leds.h |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/leds.h b/include/linux/leds.h
index d41ccb5..d3a73f5 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -123,7 +123,7 @@ extern void ledtrig_ide_activity(void);
*/
struct led_info {
const char *name;
- char*default_trigger;
+ const char *default_trigger;
int flags;
};
@@ -135,7 +135,7 @@ struct led_platform_data {
/* For the leds-gpio driver */
struct gpio_led {
const char *name;
- char *default_trigger;
+ const char *default_trigger;
unsignedgpio;
u8 active_low;
};
--
1.5.4.3
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
On Fri, 1 Aug 2008, Timur Tabi wrote: On Thu, Jul 31, 2008 at 6:32 PM, Trent Piepho [EMAIL PROTECTED] wrote: The real problem, which kept me from making a patch to do this months ago, is that the source clock that the I2C freq divider applies to is different for just about every MPC8xxx platform. Not just a different value, but a totally different internal clock. Sometimes is CCB, sometimes CCB/2, sometimes tsec2's clock, etc. On which SOC is it the tesc2 clock? 834x Sometimes the two i2c controllers don't even have the same clock. On which platform is that the case? I thought I had all 8[356]xx boards covered. Did I miss some? All 83xx other than 832x. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
On Fri, 1 Aug 2008, Jon Smirl wrote:
I don't like the third choice. Keep a simple Linux driver for i2c and
the platform, and then move all of the messy code into uboot. BTW,
the messy code is about 10 lines. It's going to take more than 10
lines to hide those 10 lines.
It's not being _moved_ to u-boot, it's already there. U-boot needs it
because u-boot uses i2c.
It would need to be duplicated in linux, and then both u-boot and linux
would need to be updated each time a new platform is added.
It's pretty much a given that a u-boot binary will only work on one
platform. The same is not true with a compiled kernel. That makes it
harder to all the platform specific stuff in linux. It's a little more
than 10 lines too. Keep in mind that accessing all these devices registers
isn't as easy in linux as u-boot. In linux you have to look up the
register location in device tree and map the registers. All this code uses
a system clock as a starting point, which u-boot already passes to linux.
#if defined(CONFIG_MPC83XX)
volatile immap_t *im = (immap_t *) CFG_IMMR;
sccr = im-clk.sccr;
#if defined(CONFIG_MPC834X) || defined(CONFIG_MPC831X) ||
defined(CONFIG_MPC837X)
switch ((sccr SCCR_TSEC1CM) SCCR_TSEC1CM_SHIFT) {
case 0:
tsec1_clk = 0;
break;
case 1:
tsec1_clk = csb_clk;
break;
case 2:
tsec1_clk = csb_clk / 2;
break;
case 3:
tsec1_clk = csb_clk / 3;
break;
default:
/* unkown SCCR_TSEC1CM value */
return -2;
}
#endif
#if defined(CONFIG_MPC834X) || defined(CONFIG_MPC837X) ||
defined(CONFIG_MPC8315)
switch ((sccr SCCR_TSEC2CM) SCCR_TSEC2CM_SHIFT) {
case 0:
tsec2_clk = 0;
break;
case 1:
tsec2_clk = csb_clk;
break;
case 2:
tsec2_clk = csb_clk / 2;
break;
case 3:
tsec2_clk = csb_clk / 3;
break;
default:
/* unkown SCCR_TSEC2CM value */
return -4;
}
#elif defined(CONFIG_MPC8313)
tsec2_clk = tsec1_clk;
if (!(sccr SCCR_TSEC1ON))
tsec1_clk = 0;
if (!(sccr SCCR_TSEC2ON))
tsec2_clk = 0;
#endif
switch ((sccr SCCR_ENCCM) SCCR_ENCCM_SHIFT) {
case 0:
enc_clk = 0;
break;
case 1:
enc_clk = csb_clk;
break;
case 2:
enc_clk = csb_clk / 2;
break;
case 3:
enc_clk = csb_clk / 3;
break;
default:
/* unkown SCCR_ENCCM value */
return -7;
}
#if defined(CONFIG_MPC837X)
switch ((sccr SCCR_SDHCCM) SCCR_SDHCCM_SHIFT) {
case 0:
sdhc_clk = 0;
break;
case 1:
sdhc_clk = csb_clk;
break;
case 2:
sdhc_clk = csb_clk / 2;
break;
case 3:
sdhc_clk = csb_clk / 3;
break;
default:
/* unkown SCCR_SDHCCM value */
return -8;
}
#endif
#if defined(CONFIG_MPC834X)
i2c1_clk = tsec2_clk;
#elif defined(CONFIG_MPC8360)
i2c1_clk = csb_clk;
#elif defined(CONFIG_MPC832X)
i2c1_clk = enc_clk;
#elif defined(CONFIG_MPC831X)
i2c1_clk = enc_clk;
#elif defined(CONFIG_MPC837X)
i2c1_clk = sdhc_clk;
#endif
#if !defined(CONFIG_MPC832X)
i2c2_clk = csb_clk; /* i2c-2 clk is equal to csb clk */
#endif
#elif defined (CONFIG_MPC85XX)
#if defined(CONFIG_MPC8540) || defined(CONFIG_MPC8541) || \
defined(CONFIG_MPC8560) || defined(CONFIG_MPC8555)
gd-i2c1_clk = sys_info.freqSystemBus;
#elif defined(CONFIG_MPC8544)
volatile ccsr_gur_t *gur = (void *) CFG_MPC85xx_GUTS_ADDR;
/*
* On the 8544, the I2C clock is the same as the SEC clock. This can be
* either CCB/2 or CCB/3, depending on the value of cfg_sec_freq. See
* 4.4.3.3 of the 8544 RM. Note that this might actually work for all
* 85xx, but only the 8544 has cfg_sec_freq, so it's unknown if the
* PORDEVSR2_SEC_CFG bit is 0 on all 85xx boards that are not an 8544.
*/
if (gur-pordevsr2 MPC85xx_PORDEVSR2_SEC_CFG)
gd-i2c1_clk = sys_info.freqSystemBus / 3;
else
gd-i2c1_clk = sys_info.freqSystemBus / 2;
#else
/* Most 85xx SOCs use CCB/2, so this is the default behavior. */
gd-i2c1_clk = sys_info.freqSystemBus / 2;
#endif
gd-i2c2_clk = gd-i2c1_clk;
#elif defined(CONFIG_MPC86XX)
#ifdef CONFIG_MPC8610
gd-i2c1_clk = sys_info.freqSystemBus;
#else
gd-i2c1_clk = sys_info.freqSystemBus /
Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
On Thu, 31 Jul 2008, Timur Tabi wrote: Jon Smirl wrote: Aren't the tables in the manual there just to make it easy for a human to pick out the line they want? For a computer you'd program the formula that was used to create the tables. Actually, the tables in the manuals are just an example of what can be programmed. They don't cover all cases. The tables assume a specific value of DFSR. For 8xxx, you want to use AN2919 to figure out how to really program the device. The table I generated for U-Boot is based on the calculations outlined in AN2919. I debated trying to implement the actual algorithm, but decided that pre-calculating the values was better. The algorithm is very convoluted. And I decided to program the algorithm. It's not that complex and overall compiles to less total size than the table method. It doesn't involve some black box table either. The real problem, which kept me from making a patch to do this months ago, is that the source clock that the I2C freq divider applies to is different for just about every MPC8xxx platform. Not just a different value, but a totally different internal clock. Sometimes is CCB, sometimes CCB/2, sometimes tsec2's clock, etc. Sometimes the two i2c controllers don't even have the same clock. I didn't see any easy way to get this information. In U-Boot I get it from the global board info struct, but I didn't see anything like this in Linux. I agree that it took me half an hour to figure out the formula that was needed to compute the i2s clocks, but once you figure out the formula it solves all of the cases and no one needs to read the manual any more. The i2c formula may even need a small loop which compares different solutions looking for the smallest error term. But it's a small space to search. Yes, I needed a loop. The divider ends up having the form (a + c) b, where a is from some set of eight integers between 2 and 15, b has a range of like 5 to 12, and c is usually zero. So, I find the divider I want to get, loop over each b, shift the divider right by b and find the closest (a+c) to that, calculate the error, and then use the best one. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
On Thu, 31 Jul 2008, Jon Smirl wrote:
As for the source clock, how about creating a new global like
ppc_proc_freq called ppc_ipb_freq. The platform code can then set the
right clock value into the variable. For mpc8 get it from uboot.
mpc5200 can easily compute it from ppc_proc_freq and checking how the
ipb divider is set. That will move the clock problem out of the i2c
driver.
There is a huge variation in where the I2C source clock comes from.
Sometimes it's the system bus, sometimes ethernet, sometimes SEC, etc. If
I look at u-boot (which might not be entirely correct or complete), I see:
83xx: 5 different clock sources
85xx: 3 different clock sources
86xx: 2 different clock sources
But there's more. Sometimes the two I2C controllers don't use the same
clock! So even if you add 10 globals with different clocks, and then add
code to the mpc i2c driver so if can figure out which one to use given the
platform, it's still not enough because you need to know which controller
the device node is for.
IMHO, what Timur suggested of having u-boot put the source clock into the
i2c node makes the most sense. U-boot has to figure this out, so why
duplicate the work?
Here's my idea:
[EMAIL PROTECTED] {
compatible = fsl-i2c;
bus-frequency = 10;
/* Either */
source-clock-frequency = 0;
/* OR */
source-clock = ccb;
};
bus-frequency is the desired frequency of the i2c bus, i.e. 100 kHz or 400
kHz usually. source-clock-frequency is the the source clock to this i2c
controller. U-Boot can fill this in since it already knows it anyway. Or,
instead of source-clock-frequency, source-clock can be specified as a
phandle to a device which has the clock to use. This could be useful if
Linux can change the clock frequency.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
On Thu, 31 Jul 2008, Grant Likely wrote:
On Thu, Jul 31, 2008 at 09:54:48PM +0200, Wolfgang Grandegger wrote:
Thinking more about it, it would be best to add the property
i2c-clock-divider to the soc node and implement fsl_get_i2c_freq() in
a similar way:
[EMAIL PROTECTED] {
#address-cells = 1;
#size-cells = 1;
device_type = soc;
ranges = 0x0 0xe000 0x10;
reg = 0xe000 0x1000; // CCSRBAR 1M
bus-frequency = 0;
i2c-clock-divider = 2;
U-Boot could then fixup that value like bus-frequency() and the i2c-mpc
driver simply calls fsl_get_i2c_freq().
Except the i2c clock isn't always a based on an interger divider of the CCB
frequency. What's more, it's not always the same for both i2c controllers.
Suppose i2c #1 uses CCB times 2/3 and i2c #2 uses CCB/2, how would
fsl_get_i2c_freq() figure that out from bus-frequency and
i2c-clock-divider?
Ugh. This is specifically related to the i2c device, so please place
the property in the i2c device. Remember, device tree design is not
about what will make the implementation simplest, but rather about what
describes the hardware in the best way.
Now, if you can argue that i2c-clock-frequency is actually a separate
clock domain defined at the SoC level, not the i2c device level, then I
will change my opinion.
The i2c controller just uses some system clock that was handy. For each
chip, the designers consult tea leaves to choose a system clock at random
to connect to the i2c controller.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
On Thu, 31 Jul 2008, Grant Likely wrote: I'm a bit confused. The frequency of the I2C source clock and the real I2C clock frequency are two different things. The first one is common for all I2C devices, the second can be different. What properties would you like to use for defining both? How is the divider controlled? Is it a fixed property of the SoC? Usually. a shared register setting? Sometimes. or a register setting within the i2c device? Never. Thinking of it as the CCB divided by 1, 2, 3 isn't really right. It's just some internal clock that I2C was connected to. Sometimes it's the clock for the ethernet block of the SoC. Sometimes it comes from some other block. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
On Thu, 31 Jul 2008, Timur Tabi wrote: Wolfgang Grandegger wrote: U-Boot does not (yet) use the FDT and it has therefore to use that ugly code to derive the I2C input clock frequency. I think its completely legal to put that hardware specific information into the FDT and get rid of such code. Huh? U-Boot initializes several fields and creates several properties in the FDT today, so what's wrong with adding another one? The clock frequencies have always been calculated by U-Boot, because putting them in the device tree is a bad idea. I think Wolfgang means u-boot doesn't _read_ information from the device tree. Put the I2C source clock into the device tree and have u-boot read it in. But that simply doesn't work at all. The i2c source clock isn't constant. It's not a constant divider from the core clock either. Flip a dip switch, and the divider changes. But that's irrelevant. U-boot needs to configure the i2c controller to read SPD data from the DIMM's EPROM, which is necessary to get DRAM working. That happens long long before the FDT is loaded via TFTP, y-modem, NFS, or from disk or flash. Might as well have u-boot get the i2c source clock via Linux's sysfs. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [i2c] [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
On Thu, 31 Jul 2008, Jon Smirl wrote:
Here's my idea:
[EMAIL PROTECTED] {
compatible = fsl-i2c;
bus-frequency = 10;
/* Either */
source-clock-frequency = 0;
/* OR */
source-clock = ccb;
};
Can't we hide a lot of this on platforms where the source clock is not
messed up? For example the mpc5200 doesn't need any of this, the
needed frequency is already available in mpc52xx_find_ipb_freq().
mpc5200 doesn't need any uboot change.
Next would be normal mpc8xxx platforms where i2c is driven by a single
clock, add a uboot filled in parameter in the root node (or I think it
can be computed off of the ones uboot is already filling in). make a
mpc8xxx_find_i2c_freq() function. May not need to change device tree
and uboot.
Finally use this for those days when the tea leaves were especially
bad. Both a device tree and uboot change.
If you have to support clock speed in the i2c node anyway, what's the point
of the other options?
Except the i2c clock isn't always a based on an integer divider of the CCB
frequency. What's more, it's not always the same for both i2c controllers.
Suppose i2c #1 uses CCB times 2/3 and i2c #2 uses CCB/2, how would
fsl_get_i2c_freq() figure that out from bus-frequency and
i2c-clock-divider?
If you get the CCB frequency from uboot and know the chip model, can't
you compute these in the platform code? Then make a
mpc8xxx_find_i2c_freq(cell_index).
You need a bunch of random other device registers (SEC, ethernet, sdhc,
etc.) too.
I don't see why we want to go through the trouble of having uboot tell
us things about a chip that are fixed in stone. Obviously something
like the frequency of the external crystal needs to be passed up, but
why pass up stuff that can be computed (or recovered by reading a
register)?
One could also say that if U-boot has to have the code and already
calculated the value, why duplicate the code and the calculation in Linux?
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 2/2] leds: Support OpenFirmware led bindings
On Sat, 26 Jul 2008, Grant Likely wrote: On Fri, Jul 25, 2008 at 02:01:45PM -0700, Trent Piepho wrote: Add bindings to support LEDs defined as of_platform devices in addition to the existing bindings for platform devices. +- gpios : Should specify the LED GPIO. Question: it is possible/desirable for a single LED to be assigned multiple GPIO pins? Say, for a tri-color LED? (I'm fishing for opinions; I really don't know if it would be a good idea or not) Good question. The Linux LED layer has no concept of multi-color LEDs, so it's more difficult that just adding support to the gpio led driver. I have a device with a tri-color red/green/orange LED and this posed some difficulty. It's defined as independent red and greed LEDs, which is mostly fine, except I wanted it to flash orange. I can make both the red LED and green LED flash, but there is nothing to insure their flashing remains in sync. Other OF bindings allow multiple GPIOs to be listed in a gpios property, so that's not a problem if someone wants to do that. There would need to be a way to define what the gpios mean. I don't think it's worthwhile to come up with a binding for that until there is a real user. +- function : (optional) This parameter, if present, is a string + defining the function of the LED. It can be used to put the LED + under software control, e.g. Linux LED triggers like heartbeat, + ide-disk, and timer. Or it could be used to attach a hardware + signal to the LED, e.g. a SoC that can configured to put a SATA + activity signal on a GPIO line. This makes me nervous. It exposes Linux internal implementation details into the device tree data. If you want to have a property that describes the LED usage, then the possible values and meanings should be documented here. Should it be a linux specific property then? I could list all the current linux triggers, but enumerating every possible function someone might want to assign to an LED seems hopeless. +led.default_trigger = +of_get_property(child, linux,default-trigger, NULL); This isn't in the documented binding. I assume that you mean 'function' here? Looks like I emailed the wrong patch file. That should be changed to function and there are a few cosmetic changes that are missing. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 2/2] leds: Support OpenFirmware led bindings
On Mon, 28 Jul 2008, Anton Vorontsov wrote:
On Mon, Jul 28, 2008 at 11:09:14AM -0600, Grant Likely wrote:
[...]
+- function : (optional) This parameter, if present, is a string
+ defining the function of the LED. It can be used to put the LED
+ under software control, e.g. Linux LED triggers like heartbeat,
+ ide-disk, and timer. Or it could be used to attach a hardware
+ signal to the LED, e.g. a SoC that can configured to put a SATA
+ activity signal on a GPIO line.
This makes me nervous. It exposes Linux internal implementation details
into the device tree data. If you want to have a property that
describes the LED usage, then the possible values and meanings should be
documented here.
Should it be a linux specific property then? I could list all the current
linux triggers, but enumerating every possible function someone might want
to assign to an LED seems hopeless.
I'd rather see the device tree provide 'hints' toward the expected usage
and if a platform needs something specific, then the platform specific
code should setup the trigger.
Maybe we can encode leds into devices themselves, via phandles?
How will this work for anything besides ide activity? For example, flashing,
heartbeat, default on, overheat, fan failed, kernel panic, etc.
And then the OF GPIO LEDs driver could do something like:
char *ide_disk_trigger_compatibles[] = {
fsl,sata,
ide-generic,
...
};
Everytime someone added a new ide driver, this table would have to be updated.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 2/2] leds: Support OpenFirmware led bindings
On Mon, 28 Jul 2008, Anton Vorontsov wrote:
On Mon, Jul 28, 2008 at 11:26:28AM -0700, Trent Piepho wrote:
On Mon, 28 Jul 2008, Anton Vorontsov wrote:
On Mon, Jul 28, 2008 at 11:09:14AM -0600, Grant Likely wrote:
[...]
+- function : (optional) This parameter, if present, is a string
+ defining the function of the LED. It can be used to put the LED
+ under software control, e.g. Linux LED triggers like heartbeat,
+ ide-disk, and timer. Or it could be used to attach a hardware
+ signal to the LED, e.g. a SoC that can configured to put a SATA
+ activity signal on a GPIO line.
This makes me nervous. It exposes Linux internal implementation details
into the device tree data. If you want to have a property that
describes the LED usage, then the possible values and meanings should be
documented here.
Should it be a linux specific property then? I could list all the current
linux triggers, but enumerating every possible function someone might want
to assign to an LED seems hopeless.
I'd rather see the device tree provide 'hints' toward the expected usage
and if a platform needs something specific, then the platform specific
code should setup the trigger.
Maybe we can encode leds into devices themselves, via phandles?
How will this work for anything besides ide activity? For example, flashing,
heartbeat, default on, overheat, fan failed, kernel panic, etc.
Everything is possible, but will look weird. For example,
Default on (power led) could be encoded in the root node.
fan and overheat in a PM controller's node.
Kernel panic in the chosen node.
What about flashing? What if the sensor chip isn't an OF device?
And then the OF GPIO LEDs driver could do something like:
char *ide_disk_trigger_compatibles[] = {
fsl,sata,
ide-generic,
...
};
Everytime someone added a new ide driver, this table would have to be
updated.
Yes. device_type would be helpful here. :-)
Well, otherwise, we could provide a trigger map in the chosen node:
chosen {
/* leds map: default-on, ide-disk, nand-disk, panic */
linux,leds = green_led red_led 0 0;
};
What if you have multiple leds that you want to be default on? What happens
when new functions are added?
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 1/2] leds: make the default trigger name const
On Sun, 27 Jul 2008, Stephen Rothwell wrote: On Sat, 26 Jul 2008 20:08:57 -0600 Grant Likely [EMAIL PROTECTED] wrote: On Fri, Jul 25, 2008 at 02:01:44PM -0700, Trent Piepho wrote: The default_trigger fields of struct gpio_led and thus struct led_classdev are pretty much always assigned from a string literal, which means the string can't be modified. Which is fine, since there is no reason to modify the string and in fact it never is. But they should be marked const to prevent such code from being added, to prevent warnings if -Wwrite-strings is used and when assigned from a constant string other than a string literal (which produces a warning under current kernel compiler flags), and for general good coding practices. Signed-off-by: Trent Piepho [EMAIL PROTECTED] Acked-by: Grant Likely [EMAIL PROTECTED] I would ack this as well, except I am not sure what tree this patch is against. In the current powerpc next tree, It was against powerpc next from Jul 22nd, current at the time I made the patch. It looks like that file has changed in the last few days. There is a patch from Anton Vorontsov, leds: mark led_classdev.default_trigger as const, which adds const to one of the structs I modified, but doesn't get the other one (struct gpio_led). Then another patch from Nate Case added a new LED chip driver, and the platform data for this driver was added as generic led platform data, which I don't entirely agree with. And this new struct didn't make default_trigger const, probably because it was just copied from the gpio led platform data with some fields removed (so it's not really that generic then, is it?). I'll send an updated patch for current powerpc next. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH v2] leds: make the default trigger name const
The default_trigger fields of struct gpio_led and thus struct led_classdev
are pretty much always assigned from a string literal, which means the
string can't be modified. Which is fine, since there is no reason to
modify the string and in fact it never is.
But they should be marked const to prevent such code from being added, to
prevent warnings if -Wwrite-strings is used and when assigned from a
constant string other than a string literal (which produces a warning under
current kernel compiler flags), and for general good coding practices.
Signed-off-by: Trent Piepho [EMAIL PROTECTED]
---
include/linux/leds.h |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 519df72..defe693 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -48,7 +48,7 @@ struct led_classdev {
struct device *dev;
struct list_head node; /* LED Device list */
- char*default_trigger; /* Trigger to use */
+ const char *default_trigger; /* Trigger to use */
#ifdef CONFIG_LEDS_TRIGGERS
/* Protects the trigger data below */
@@ -121,7 +121,7 @@ extern void ledtrig_ide_activity(void);
/* For the leds-gpio driver */
struct gpio_led {
const char *name;
- char *default_trigger;
+ const char *default_trigger;
unsignedgpio;
u8 active_low;
};
--
1.5.4.3
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC PATCH] of_gpio: implement of_get_gpio_flags()
On Fri, 25 Jul 2008, Anton Vorontsov wrote: The name seems a bit unfortunate though, because one could read the function as it gets gpio flags only (though, we might implement this instead, but this way average driver will end up with parsing gpios = twice). It is kind of a confusing name. I'd be tempted to just change of_get_gpio, it's a new function and it's only called from four places so it's not that hard to change. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v3] leds: implement OpenFirmare GPIO LED driver
Here are my patches for the OF bindings. The first is just a tiny change to
the leds code to silence a warning. The second is the real patch.
The leds-gpio driver gets two sub-options in Kconfig, one for platform
device support and one for openfirmware platform device support.
There is support for setting an LED trigger. I didn't include support for
active-low or initial brightness, but I think those would be good features
to add. See below for more on that.
Since each device node can describe multiple leds, I used gpio-leds
instead of gpio-led for the compatible property.
Examples of the dts syntax:
leds {
compatible = gpio-leds;
hdd {
label = IDE activity;
gpios = mcu_pio 0 0;
function = ide-disk;
};
};
run-control {
compatible = gpio-leds;
red {
gpios = mpc8572 6 0;
};
green {
gpios = mpc8572 7 0;
};
}
On Fri, 18 Jul 2008, Anton Vorontsov wrote:
Later I tried to use aliases for default-trigger.
http://ozlabs.org/pipermail/linuxppc-dev/2008-June/057244.html
I doesn't seem to me that the alias method will work very well. If I want
an LED that starts on, I need to add code to the kernel to support an
led-on-while-kernel-boots alias? And if I want red green leds to flash
while booting, I need to add aliases led-flashing-while-kernel-boots-1 and
led-flashing-while-kernel-boots-2, since you can't assign two leds to the
same alias?
As for linux,default-brightness... we don't need this since now we
have default-on trigger.
Except we can't use triggers
There is an issue with the default-on trigger, besides requiring led
triggers be turned on and the extra code that needs. It causes the led to
have a glitch in it. Suppose the LED is already on from reset or the boot
loader. When the gpio is allocated, it's allocated with an initial value of
off. Then, later, it's turned back on when the trigger runs. But say the
trigger doesn't run?
Here's a real example. I have an LED that comes on the board powers on. It
was supposed to turn off when the kernel has finished booting. But suppose
the kernel panics between the gpio getting lowered when the led is added and
the trigger turning it back on? Now the LED is off and it appears the
problem happened after the kernel finished booting, but really it happened
during the kernel boot. In an embedded system with no console, that's quite
a bit of misinformation. It can be entirely avoided by changing just three
lines of code with the leds-gpio driver to add an option to start the led
on.
It could also be the case that the gpio the led is on also triggers some
other piece of hardware. An alarm that's on the same line as a fault led
for example. The glitch created by the default-on trigger could be
unacceptable in that case.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH 1/2] leds: make the default trigger name const
The default_trigger fields of struct gpio_led and thus struct led_classdev
are pretty much always assigned from a string literal, which means the
string can't be modified. Which is fine, since there is no reason to
modify the string and in fact it never is.
But they should be marked const to prevent such code from being added, to
prevent warnings if -Wwrite-strings is used and when assigned from a
constant string other than a string literal (which produces a warning under
current kernel compiler flags), and for general good coding practices.
Signed-off-by: Trent Piepho [EMAIL PROTECTED]
---
include/linux/leds.h |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 519df72..defe693 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -48,7 +48,7 @@ struct led_classdev {
struct device *dev;
struct list_head node; /* LED Device list */
- char*default_trigger; /* Trigger to use */
+ const char *default_trigger; /* Trigger to use */
#ifdef CONFIG_LEDS_TRIGGERS
/* Protects the trigger data below */
@@ -121,7 +121,7 @@ extern void ledtrig_ide_activity(void);
/* For the leds-gpio driver */
struct gpio_led {
const char *name;
- char *default_trigger;
+ const char *default_trigger;
unsignedgpio;
u8 active_low;
};
--
1.5.4.3
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH 2/2] leds: Support OpenFirmware led bindings
Add bindings to support LEDs defined as of_platform devices in addition to
the existing bindings for platform devices.
New options in Kconfig allow the platform binding code and/or the
of_platform code to be turned on. The of_platform code is of course only
available on archs that have OF support.
The existing probe and remove methods are refactored to use new functions
create_gpio_led(), to create and register one led, and delete_gpio_led(),
to unregister and free one led. The new probe and remove methods for the
of_platform driver can then share most of the common probe and remove code
with the platform driver.
The suspend and resume methods aren't shared, but they are very short. The
actual led driving code is the same for LEDs created by either binding.
The OF bindings are based on patch by Anton Vorontsov
[EMAIL PROTECTED]. They have been extended to allow multiple LEDs
per device.
Signed-off-by: Trent Piepho [EMAIL PROTECTED]
---
Documentation/powerpc/dts-bindings/gpio/led.txt | 44 -
drivers/leds/Kconfig| 21 ++-
drivers/leds/leds-gpio.c| 225 ++-
3 files changed, 236 insertions(+), 54 deletions(-)
diff --git a/Documentation/powerpc/dts-bindings/gpio/led.txt
b/Documentation/powerpc/dts-bindings/gpio/led.txt
index ff51f4c..ed01297 100644
--- a/Documentation/powerpc/dts-bindings/gpio/led.txt
+++ b/Documentation/powerpc/dts-bindings/gpio/led.txt
@@ -1,15 +1,39 @@
-LED connected to GPIO
+LEDs connected to GPIO lines
Required properties:
-- compatible : should be gpio-led.
-- label : (optional) the label for this LED. If omitted, the label is
- taken from the node name (excluding the unit address).
-- gpios : should specify LED GPIO.
+- compatible : should be gpio-leds.
-Example:
+Each LED is represented as a sub-node of the gpio-leds device. Each
+node's name represents the name of the corresponding LED.
[EMAIL PROTECTED] {
- compatible = gpio-led;
- label = hdd;
- gpios = mcu_pio 0 1;
+LED node properties:
+- gpios : Should specify the LED GPIO.
+- label : (optional) The label for this LED. If omitted, the label
+ is taken from the node name (excluding the unit address).
+- function : (optional) This parameter, if present, is a string
+ defining the function of the LED. It can be used to put the LED
+ under software control, e.g. Linux LED triggers like heartbeat,
+ ide-disk, and timer. Or it could be used to attach a hardware
+ signal to the LED, e.g. a SoC that can configured to put a SATA
+ activity signal on a GPIO line.
+
+Examples:
+
+leds {
+ compatible = gpio-leds;
+ hdd {
+ label = IDE activity;
+ gpios = mcu_pio 0 0;
+ function = ide-disk;
+ };
};
+
+run-control {
+ compatible = gpio-leds;
+ red {
+ gpios = mpc8572 6 0;
+ };
+ green {
+ gpios = mpc8572 7 0;
+ };
+}
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 86a369b..8344256 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -109,7 +109,26 @@ config LEDS_GPIO
help
This option enables support for the LEDs connected to GPIO
outputs. To be useful the particular board must have LEDs
- and they must be connected to the GPIO lines.
+ and they must be connected to the GPIO lines. The LEDs must be
+ defined as platform devices and/or OpenFirmware platform devices.
+ The code to use these bindings can be selected below.
+
+config LEDS_GPIO_PLATFORM
+ bool Platform device bindings for GPIO LEDs
+ depends on LEDS_GPIO
+ default y
+ help
+ Let the leds-gpio driver drive LEDs which have been defined as
+ platform devices. If you don't know what this means, say yes.
+
+config LEDS_GPIO_OF
+ bool OpenFirmware platform device bindings for GPIO LEDs
+ depends on LEDS_GPIO OF_DEVICE
+ default y
+ help
+ Let the leds-gpio driver drive LEDs which have been defined as
+ of_platform devices. For instance, LEDs which are listed in a dts
+ file.
config LEDS_CM_X270
tristate LED Support for the CM-X270 LEDs
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index b13bd29..f10f123 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -71,11 +71,52 @@ static int gpio_blink_set(struct led_classdev *led_cdev,
return led_dat-platform_gpio_blink_set(led_dat-gpio, delay_on,
delay_off);
}
-static int gpio_led_probe(struct platform_device *pdev)
+static int __devinit create_gpio_led(const struct gpio_led *template,
+ struct gpio_led_data *led_dat, struct device *parent,
+ int (*blink_set)(unsigned, unsigned long *, unsigned long *))
+{
+ int ret;
+
+ ret = gpio_request(template-gpio, template-name);
+ if (ret 0)
+ return ret;
+
+ led_dat
Re: PIXIS gpio controller and gpio flags
On Wed, 23 Jul 2008, Anton Vorontsov wrote: On Mon, Jul 21, 2008 at 02:12:20PM -0700, Trent Piepho wrote: On Mon, 21 Jul 2008, Anton Vorontsov wrote: On Sat, Jul 19, 2008 at 02:08:01PM -0700, Trent Piepho wrote: It doesn't look like you have any way to unset the active low flag. What if I unload the leds-gpio driver (or another gpio user) and then try to use the gpio with something else? The active low flag is stuck on! Why would you want to unset the flags? It is specified in the device tree, and can't be changed. Specifying different flags for the same GPIO would be an error (plus, Linux forbids shared gpios, so you simply can't specify one gpio for several devices). You can't use the same gpio for two different things at the same time, but you can load a driver, unload it, and then load another. Hm.. yeah, this might happen. Now I tend to think that transparent active-low handling wasn't that good idea after all. So, something like of_gpio_is_active_low(device_node, gpioidx) should be implemented instead. This function will parse the gpio's = flags. Please speak up if you have any better ideas though. The flags could be provided via of_get_gpio. E.g., something like of_get_gpio(, u32 *flags), and if flags is non-NULL the gpio flags are left there. of_get_gpio already has the flags and some other of_get_* functions return multiple things like this. Or just have an active low property for the led: led.active_low = !!of_get_property(np, active-low, NULL); Pretty simple, just one line of code. At least if one looks just at leds-gpio, that's obviously the simplest way. Is active low a property of the led or a property of the gpio? I guess one could argue either way. It seems like putting one line of code leds-gpio driver is better than putting much more complex code into the gpio controller driver. And each gpio controller has to have that code too. Now you could say that each gpio user needing to support inverting gpios is a lot of code too, but I don't think it's necessary. Active low LEDs are common since gpios can usually sink more current than they can source. But other gpio users, like the I2C, SPI, MDIO drivers etc., haven't had a need to support inverting each signal. I'm also loathe to add software gpio inverting to my mpc8572 gpio driver. In addition to some LEDs there is also a bit-banged JTAG bus on the CPU GPIOs. I had to go to great lengths to get it fast enough and each instruction added to a gpio operation is going to cost me MHz. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: PIXIS gpio controller and gpio flags
On Mon, 21 Jul 2008, Anton Vorontsov wrote: On Sat, Jul 19, 2008 at 02:08:01PM -0700, Trent Piepho wrote: It doesn't look like you have any way to unset the active low flag. What if I unload the leds-gpio driver (or another gpio user) and then try to use the gpio with something else? The active low flag is stuck on! Why would you want to unset the flags? It is specified in the device tree, and can't be changed. Specifying different flags for the same GPIO would be an error (plus, Linux forbids shared gpios, so you simply can't specify one gpio for several devices). You can't use the same gpio for two different things at the same time, but you can load a driver, unload it, and then load another. Most gpio users, including leds-gpio, can handle gpios being busy. If leds-gpio can't get one of the gpios, it rolls back all the leds it did create, doesn't drive the device and returns EBUSY. Except with of_get_gpio() setting the flags, it will change the flags out from under whatever had the gpio already allocated! You're still assuming that something was allocated. It wasn't. The flag was set, and it should not change. It is irrelevant if request() failed or not. Another driver has requested the gpio with active low NOT set. Then the led driver looks up the property with active low set, which changes the flags. When it tries to request the gpio it fails since the gpio is already in use. The led driver handles this failure. Except the active low flag is now set and the driver that was using the gpio before will now mysteriously stop working. Perhaps by erasing flash instead of reading it or something nasty like that. Is this the proper way to handle a dts mistake that has two drivers trying to use the same gpio? Without the gpio flags getting set when looking up the gpio, this situation is handled without any difficulty. And is having a gpio used twice in the device tree really a mistake? What if there are two different devices that can be attached to the gpios? For example, an LED bank that can be unplugged and a serial console attached in its place, sharing the same connector and gpios. Other than gpio flags getting set when looking up a gpio, it works fine to list both devices in the device tree as long as userspace has the correct driver loaded first. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: PIXIS gpio controller and gpio flags
On Fri, 18 Jul 2008, Anton Vorontsov wrote:
+int px_gpio_xlate(struct of_gpio_chip *of_gc, struct device_node *np,
+ const void *gpio_spec)
+{
+ if (gpio[1] PX_GPIO_FLAG_ACTIVE_LOW)
+ px_gc-active_low |= pin2mask(*gpio);
You have a race here. What if px_gpio_xlate() is called at the same time
for different gpios? This is an easy one to fix, since you can use the
atomic bitops.
It doesn't look like you have any way to unset the active low flag. What if
I unload the leds-gpio driver (or another gpio user) and then try to use the
gpio with something else? The active low flag is stuck on! It doesn't show
in sysfs or debugfs either. That could be very confusing.
I also wonder if it's ok to have the xlate function do flag setting?
of_get_property() just gets the property, it doesn't allocate it. Same with
of_get_address() and of_get_pci_address(), they don't actually allocate or
map an address, they just get the value. of_get_gpio() doesn't allocate the
gpio, that gets done later with gpio_request(). It seems like what it's
supposed to do is just get the translated value of the gpio property.
Except, your pixis gpio xlate function sets the gpio's flags. What if one
wants to just look up a gpio number, but not allocate it? The flags will
still get set.
Most gpio users, including leds-gpio, can handle gpios being busy. If
leds-gpio can't get one of the gpios, it rolls back all the leds it did
create, doesn't drive the device and returns EBUSY. Except with
of_get_gpio() setting the flags, it will change the flags out from under
whatever had the gpio already allocated!
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v3] leds: implement OpenFirmare GPIO LED driver
On Fri, 18 Jul 2008, Anton Vorontsov wrote:
On Thu, Jul 17, 2008 at 01:18:18PM -0700, Trent Piepho wrote:
Basically what I did then in my patch then, refactor leds-gpio so most of
it is shared and there is a block of code that does platform binding and
another block that does of_platform binding.
Ok. I must admit I'm quite burned out with OF gpio-leds. I was posting the
bindings since April, probably four or five times. Last time a week ago,
IIRC.
During the months I received just a few replies, one from Grant (Looks
good to me.), few from Segher (with a lot of criticism, that I much
appreciated and tried to fix all spotted issues), and one from Laurent
(about active-low LEDs).
I'm sorry, I never saw those emails. Were they all to linuxppc-dev? I hate
reading that list, gmane, marc, and lkml.org don't archive it and
mail-archive.com isn't nearly as nice.
Is this the last version?
http://www.mail-archive.com/linuxppc-dev@ozlabs.org/msg18355.html
Why did you get rid of linux,default-trigger and the active-low property?
I couldn't find any discussion about this. When I first saw your current
patch I was going to add them, but I see you already had them and then
removed them.
I'd like to replace my led-hack stg patch with this, but I need
default-trigger to do that. I don't need active-low or default-brightness,
but they seem like a good idea. Actually, if you look closely at the patch
I posted, you'll see I had modified the existing leds-gpio driver to add a
default-brightness feature, though I don't need it anymore. The requirement
changed from having an led on during kernel boot to having it flash. I
have another hack for that, since there is no way to pass parameters to a
trigger.
leds {
compatible = gpio-led;
[EMAIL PROTECTED] {
gpios = mpc8572 6 0;
label = red;
};
[EMAIL PROTECTED] {
gpios = mpc8572 7 0;
label = green;
};
};
I like this. Or better what Grant suggested, i.e. move label to node
name.
Ok, I used that. It's like the new way partitions are defined in NOR flash
OF bindings. My first example was more like the old way. The led name can
come from a label property or if there is none then the node name is used.
I don't think you can have colons or spaces in node names. I don't have a
default trigger property, but I'd really like to have that. I could add an
active low flag too. Maybe compatible should be gpio-leds, since you can
define more than one?
The patch is done and works for me. One can select platform device and/or
of_platform device support via Kconfig. I'll port it to the git kernel and
post it soon.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v3] leds: implement OpenFirmare GPIO LED driver
On Thu, 17 Jul 2008, Anton Vorontsov wrote: On Wed, Jul 16, 2008 at 10:13:06PM -0700, Trent Piepho wrote: Ok, how about adding code the existing leds-gpio driver so that it can creates LEDs from of_platform devices too? Few comments below. I've made a patch to do this and it works ok. The code added to leds-gpio is about half what was involved in Anton's new driver. This isn't true. Your new driver was 194 lines, not counting docs or Kconfig. My patch added about 104 lines to the existing leds-gpio driver. So yes, about half the code. There is still one of_platform device per led because of how the bindings work (but that could be changed with new bindings), but there are zero extra platform devices created. You didn't count extra platform driver. You can't #ifdef it. The only way you can avoid this is creating leds-gpio-base.c or something, and place the helper functions there. I guess, in terms of compiled size, the combined platform + of_platform driver is bigger than the of_platform only driver. Though it's much smaller than having both the platform only and of_platform only drivers. In terms of source code, there's less with the combined driver. I don't see why the combined leds-gpio driver can't have an ifdef for the platform part. All the platform driver specific code is already in one contiguous block. In fact, I just did it and it works fine. My LEDs from the dts were created and the LED I have as a platform device wasn't, as expected. Here's the patch, pretty simple: diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c --- a/drivers/leds/leds-gpio.c +++ b/drivers/leds/leds-gpio.c @@ -108,2 +108,3 @@ static int create_gpio_led(struct gpio_led *cur_led, +#ifdef CONFIG_LEDS_GPIO_PLATFORM static int gpio_led_probe(struct platform_device *pdev) @@ -222,2 +223,3 @@ module_init(gpio_led_init); module_exit(gpio_led_exit); +#endif /* CONFIG_LEDS_GPIO_PLATFORM */ +static int create_gpio_led(struct gpio_led *cur_led, The create_gpio_led() interface is also quite weird, since it implies that we have to pass two GPIO LED handles: struct gpio_led_data (that we allocated) and temporary struct gpio_led. And this helper function will just assign things from one struct to another, and then will register the led. It creates a thing from a template passed a pointer to a struct. This is very common, there must be hundreds of functions in the kernel that work this way. The difference is instead of allocating and returning the created thing, you pass it a blank pre-allocated thing to fill in and register. I know there is other code that works this way too. It's usually used, like it is here, so you can allocate a bunch of things at once and then register them one at a time. With OF driver I don't need struct gpio_led. Only the platform driver need this so platforms could pass gpio led info through it, while with OF we're getting all information from the device tree. The struct gpio_led is just used to pass the stats to the led creation function. It doesn't stick around. You could use local variables for the gpio and name and pass them to the create led function. Bundling them into a struct is an tiny change that lets more code be shared. +/* #ifdef CONFIG_LEDS_GPIO_OF */ Heh. Obviously the OF binding code should be conditional, selectable from kconfig if the platform has OF support. It's all in one contiguous block and that shows where the ifdef would go. I didn't think it was necessary to include the obvious kconfig patch. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v3] leds: implement OpenFirmare GPIO LED driver
On Thu, 17 Jul 2008, Grant Likely wrote:
Alternately, I would also be okay with a scheme where all LED nodes
have a common parent and an of_platform driver would bind against the
parent node; not the individual children. Then the leds-gpio driver
could be refactored to have both platform and of_platform bus
bindings.
Basically what I did then in my patch then, refactor leds-gpio so most of
it is shared and there is a block of code that does platform binding and
another block that does of_platform binding.
I didn't change the OF platform binding syntax so as not to complicate the
example, but that's easy to do. Something like:
leds {
compatible = gpio-led;
gpios = mpc8572 6 0
mpc8572 7 0;
labels = red, green;
};
Or like this, which needs a little more code to parse:
leds {
compatible = gpio-led;
[EMAIL PROTECTED] {
gpios = mpc8572 6 0;
label = red;
};
[EMAIL PROTECTED] {
gpios = mpc8572 7 0;
label = green;
};
};
I like the first better. It follows the example from the docs about how
devices with multiple gpios should work too.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v3] leds: implement OpenFirmare GPIO LED driver
On Tue, 15 Jul 2008, Anton Vorontsov wrote:
Despite leds-gpio and leds-openfirmware-gpio similar purposes, there
is not much code can be shared between the two drivers (both are mostly
driver bindings anyway).
Why can't this driver use the existing gpio-led driver? Basically, do
something like this:
of_gpio_leds_probe(...)
{
gpio = of_get_gpio(np, 0);
label = of_get_property(np, label, NULL);
struct gpio_led led = {
.name = label,
.gpio = gpio,
};
pdev = platform_device_register_simple(leds-gpio, 0, NULL, 0);
platform_device_add_data(pdev, led, sizeof(led));
}
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2] leds: implement OpenFirmare GPIO LED driver
On Tue, 15 Jul 2008, Richard Purdie wrote: On Tue, 2008-07-15 at 18:23 +0400, Anton Vorontsov wrote: Spell out openfirmware :). I initially had no idea of == openfirmware and I suspect others won't either... This would be unusually long name, that is $ find . -iname '*openfirmware*' | wc -l 0 And in contrast: drivers/video/offb.c drivers/video/nvidia/nv_of.c drivers/usb/host/ohci-ppc-of.c drivers/usb/host/ehci-ppc-of.c drivers/serial/of_serial.c drivers/mtd/maps/physmap_of.c ... So, I think we should stick with the of for consistency, while confused users may consult with Kconfig for disambiguation. The other cases don't have a gpio driver to confuse this new driver with. Lets spell it out please, the filesystems can handle the extra letters :). There's drivers/mtd/maps/physmap.c and drivers/mtd/maps/physmap_of.c, drivers/serial/of_serial.c and drivers/serial/serial_core.c, drivers/usb/host/ehci-ppc-soc.c and drivers/usb/host/ehci-ppc-of.c, etc. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v3] leds: implement OpenFirmare GPIO LED driver
on Wed, 16 Jul 2008, Grant Likely wrote:
On Wed, Jul 16, 2008 at 04:18:52PM -0700, Trent Piepho wrote:
On Tue, 15 Jul 2008, Anton Vorontsov wrote:
Despite leds-gpio and leds-openfirmware-gpio similar purposes, there
is not much code can be shared between the two drivers (both are mostly
driver bindings anyway).
Why can't this driver use the existing gpio-led driver? Basically, do
something like this:
Ugh; that means registering *2* 'struct device' with the kernel instead of
one. One as a platform device and one as an of_platform device.
It's bad enough that the LED scheme we're using for OF bindings has a
separate registration for every single LED.
Ok, how about adding code the existing leds-gpio driver so that it can creates
LEDs from of_platform devices too?
I've made a patch to do this and it works ok. The code added to leds-gpio is
about half what was involved in Anton's new driver. What I did was re-factor
the existing platform device probe function to use a new function that creates
a single led. Then a new of_platform probe function can use it too. That way
most of the probe code is shared. remove, suspend and resume aren't shared,
but they're short. And the existing code to actually drive the led gets
reused as is.
There is still one of_platform device per led because of how the bindings work
(but that could be changed with new bindings), but there are zero extra
platform devices created.
Here's an example patch. It won't apply to the git kernel as is, but should
make it clear how this works.
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index a4a2838..12e681e 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -71,11 +71,45 @@ static int gpio_blink_set(struct led_classdev *led_cdev,
return led_dat-platform_gpio_blink_set(led_dat-gpio, delay_on,
delay_off);
}
+static int create_gpio_led(struct gpio_led *cur_led,
+ struct gpio_led_data *led_dat, struct device *parent,
+ int (*blink_set)(unsigned, unsigned long *, unsigned long *))
+
+{
+ int ret;
+
+ ret = gpio_request(cur_led-gpio, cur_led-name);
+ if (ret 0)
+ return ret;
+
+ led_dat-cdev.name = cur_led-name;
+ led_dat-cdev.default_trigger = cur_led-default_trigger;
+ led_dat-gpio = cur_led-gpio;
+ led_dat-can_sleep = gpio_cansleep(cur_led-gpio);
+ led_dat-active_low = cur_led-active_low;
+ if (blink_set) {
+ led_dat-platform_gpio_blink_set = blink_set;
+ led_dat-cdev.blink_set = gpio_blink_set;
+ }
+ led_dat-cdev.brightness_set = gpio_led_set;
+ led_dat-cdev.brightness = cur_led-start_on ? LED_FULL : LED_OFF;
+
+ gpio_direction_output(led_dat-gpio,
+ led_dat-active_low ^ cur_led-start_on);
+
+ INIT_WORK(led_dat-work, gpio_led_work);
+
+ ret = led_classdev_register(parent, led_dat-cdev);
+ if (ret 0)
+ gpio_free(led_dat-gpio);
+
+ return ret;
+}
+
static int gpio_led_probe(struct platform_device *pdev)
{
struct gpio_led_platform_data *pdata = pdev-dev.platform_data;
- struct gpio_led *cur_led;
- struct gpio_led_data *leds_data, *led_dat;
+ struct gpio_led_data *leds_data;
int i, ret = 0;
if (!pdata)
@@ -87,36 +121,10 @@ static int gpio_led_probe(struct platform_device *pdev)
return -ENOMEM;
for (i = 0; i pdata-num_leds; i++) {
- cur_led = pdata-leds[i];
- led_dat = leds_data[i];
-
- ret = gpio_request(cur_led-gpio, cur_led-name);
+ ret = create_gpio_led(pdata-leds[i], leds_data[i],
+ pdev-dev, pdata-gpio_blink_set);
if (ret 0)
goto err;
-
- led_dat-cdev.name = cur_led-name;
- led_dat-cdev.default_trigger = cur_led-default_trigger;
- led_dat-gpio = cur_led-gpio;
- led_dat-can_sleep = gpio_cansleep(cur_led-gpio);
- led_dat-active_low = cur_led-active_low;
- if (pdata-gpio_blink_set) {
- led_dat-platform_gpio_blink_set =
pdata-gpio_blink_set;
- led_dat-cdev.blink_set = gpio_blink_set;
- }
- led_dat-cdev.brightness_set = gpio_led_set;
- led_dat-cdev.brightness =
- cur_led-start_on ? LED_FULL : LED_OFF;
-
- gpio_direction_output(led_dat-gpio,
- led_dat-active_low ^ cur_led-start_on);
-
- INIT_WORK(led_dat-work, gpio_led_work);
-
- ret = led_classdev_register(pdev-dev, led_dat-cdev);
- if (ret 0) {
- gpio_free(led_dat-gpio);
- goto err;
- }
}
platform_set_drvdata(pdev, leds_data);
@@ -217,3 +225,105 @@ MODULE_AUTHOR
Re: [PATCH] [Rev2] MPC5121 FEC support
On Tue, 17 Jun 2008, Scott Wood wrote: Sam Ravnborg wrote: In general when you select a symbol that has dependencies you are almost always on the wrong track. more specific options should make sure that they never select it when the dependencies aren't met. Sure, in theory that would work, but in practice this ends up being a constant source of broken builds. Use a dependency here with a sane default - then people can set it to 'n' if they really do not want this driver. Spreading selects too much is just causing you pain in the long run. I'm not sure I understand what you're looking for, but I don't see anything wrong with something like this (apart from missing help text): config FS_ENET bool select MII select PHYLIB config FS_ENET_HAS_SCC bool Freescale CPM SCC Ethernet depends on CPM1 || CPM2 select FS_ENET What prevents me from turning on FS_ENET_HAS_SCC without MII or PHYLIB? Why is FS_ENET_HAS_SCC a bool, and not tristate? ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: MMIO and gcc re-ordering issue
On Tue, 3 Jun 2008, Linus Torvalds wrote: On Tue, 3 Jun 2008, Nick Piggin wrote: Linus: on x86, memory operations to wc and wc+ memory are not ordered with one another, or operations to other memory types (ie. load/load and store/store reordering is allowed). Also, as you know, store/load reordering is explicitly allowed as well, which covers all memory types. So perhaps it is not quite true to say readl/writel is strongly ordered by default even on x86. You would have to put in some mfence instructions in them to make it so. So on x86, these could be re-ordered? writel(START_OPERATION, CONTROL_REGISTER); status = readl(STATUS_REGISTER); Well, you have to ask for WC/WC+ anyway, so it's immaterial. A driver that does that needs to be aware of it. IOW, it's a non-issue, imnsho. You need to ask for coherent DMA memory too. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: MMIO and gcc re-ordering issue
On Tue, 3 Jun 2008, Matthew Wilcox wrote: On Tue, Jun 03, 2008 at 11:47:00AM -0700, Trent Piepho wrote: On Tue, 3 Jun 2008, Linus Torvalds wrote: On Tue, 3 Jun 2008, Nick Piggin wrote: Linus: on x86, memory operations to wc and wc+ memory are not ordered with one another, or operations to other memory types (ie. load/load and store/store reordering is allowed). Also, as you know, store/load reordering is explicitly allowed as well, which covers all memory types. So perhaps it is not quite true to say readl/writel is strongly ordered by default even on x86. You would have to put in some mfence instructions in them to make it so. So on x86, these could be re-ordered? writel(START_OPERATION, CONTROL_REGISTER); status = readl(STATUS_REGISTER); You wouldn't ask for write-combining memory mapping for control or status registers. But Nick said, store/load reordering is explicitly allowed as well, which covers *all* memory types. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: MMIO and gcc re-ordering issue
On Tue, 3 Jun 2008, Nick Piggin wrote: On Monday 02 June 2008 17:24, Russell King wrote: So, can the semantics of what's expected from these IO accessor functions be documented somewhere. Please? Before this thread gets lost in the depths of time? This whole thread also ties in with my posts about mmiowb (which IMO should go away). readl/writel: strongly ordered wrt one another and other stores to cacheable RAM, byteswapping __readl/__writel: not ordered (needs mb/rmb/wmb to order with other readl/writel and cacheable operations, or io_*mb to order with one another) raw_readl/raw_writel: strongly ordered, no byteswapping __raw_readl/__raw_writel: not ordered, no byteswapping Byte-swapping vs not byte-swapping is not usually what the programmer wants. Usually your device's registers are defined as being big-endian or little-endian and you want whatever is needed to give you that. I believe that on some archs that can be either byte order, some built-in devices will change their registers to match, and so you want native endian or no swapping for these. Though that's definitely in the minority. An accessors that always byte-swaps regardless of the endianness of the host is never something I've seen a driver want. IOW, there are four ways one can defined endianness/swapping: 1) Little-endian 2) Big-endian 3) Native-endian aka non-byte-swapping 4) Foreign-endian aka byte-swapping 1 and 2 are by far the most used. Some code wants 3. No one wants 4. Yet our API is providing 3 4, the two which are the least useful. Is it enough to provide only all or none for ordering strictness? For instance on powerpc, one can get a speedup by dropping strict ordering for IO vs cacheable memory, but still keeping ordering for IO vs IO and IO vs locks. This is much easier to program for than no ordering at all. In fact, if one doesn't use coherent DMA, it's basically the same as fully strict ordering. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: MMIO and gcc re-ordering issue
On Tue, 3 Jun 2008, Matthew Wilcox wrote: On Tue, Jun 03, 2008 at 12:43:21PM -0700, Trent Piepho wrote: IOW, there are four ways one can defined endianness/swapping: 1) Little-endian 2) Big-endian 3) Native-endian aka non-byte-swapping 4) Foreign-endian aka byte-swapping 1 and 2 are by far the most used. Some code wants 3. No one wants 4. Yet our API is providing 3 4, the two which are the least useful. You've fundamentally misunderstood. readX/writeX and __readX/__writeX provide little-endian access. __raw_readX provide native-endian. If you want 2 or 4, define your own accessors. Some architectures define other accessors (eg gsc_readX on parisc is native (big) endian, and How about providing 1 and 2, and if you want 3 or 4 define your own accessors? Is it enough to provide only all or none for ordering strictness? For instance on powerpc, one can get a speedup by dropping strict ordering for IO vs cacheable memory, but still keeping ordering for IO vs IO and IO vs locks. This is much easier to program for than no ordering at all. In fact, if one doesn't use coherent DMA, it's basically the same as fully strict ordering. I don't understand why you keep talking about DMA. Are you talking about ordering between readX() and DMA? PCI proides those guarantees. I guess you haven't been reading the whole thread. The reason it started was because gcc can re-order powerpc (and everyone else's too) IO accesses vs accesses to cachable memory (but not spin-locks), which ends up only being a problem with coherent DMA. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: MMIO and gcc re-ordering issue
On Tue, 3 Jun 2008, Matthew Wilcox wrote: On Tue, Jun 03, 2008 at 12:57:56PM -0700, Trent Piepho wrote: On Tue, 3 Jun 2008, Matthew Wilcox wrote: On Tue, Jun 03, 2008 at 11:47:00AM -0700, Trent Piepho wrote: On Tue, 3 Jun 2008, Linus Torvalds wrote: On Tue, 3 Jun 2008, Nick Piggin wrote: Linus: on x86, memory operations to wc and wc+ memory are not ordered with one another, or operations to other memory types (ie. load/load and store/store reordering is allowed). Also, as you know, store/load reordering is explicitly allowed as well, which covers all memory types. So perhaps it is not quite true to say readl/writel is strongly ordered by default even on x86. You would have to put in some mfence instructions in them to make it so. So on x86, these could be re-ordered? writel(START_OPERATION, CONTROL_REGISTER); status = readl(STATUS_REGISTER); You wouldn't ask for write-combining memory mapping for control or status registers. But Nick said, store/load reordering is explicitly allowed as well, which covers *all* memory types. Then Nick is confused. PCI only defines one way to flush posted writes to a device -- doing a read from it. There's no way that reads can be allowed to pass writes (unless you've asked for it, like with write combining). But that requirement is for the PCI bridge, isn't it? It doesn't matter if the bridge will flush all posted writes before allowing a read if the CPU decides to give the bridge the read before the write. A powerpc CPU will certainly do this if you don't take any steps like telling it the memory is uncachable and guarded. I didn't think it was allowed on x86 (except with WC), but Nick seemed to say it was. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
