* [PATCH] powerpc/mm: Implement _PAGE_SPECIAL & pte_special() for 32-bit
From: Kumar Gala @ 2008-07-31 18:48 UTC (permalink / raw)
To: linuxppc-dev; +Cc: npiggin, linux-kernel
Implement _PAGE_SPECIAL and pte_special() for 32-bit powerpc. This bit will
be used by the fast get_user_pages() to differenciate PTEs that correspond
to a valid struct page from special mappings that don't such as IO mappings
obtained via io_remap_pfn_ranges().
We currently only implement this on sub-arch that support SMP or will so
in the future (6xx, 44x, FSL-BookE) and not (8xx, 40x).
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
Nick, do you forsee using _PAGE_SPECIAL for other applications that would
be of interested to non-SMP hw?
We can look at adding it into 8xx and 40x, but was being lazy as I assume
there is no point.
- k
include/asm-powerpc/pgtable-ppc32.h | 15 +++++++++++++--
1 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/include/asm-powerpc/pgtable-ppc32.h b/include/asm-powerpc/pgtable-ppc32.h
index 6fe39e3..58afaee 100644
--- a/include/asm-powerpc/pgtable-ppc32.h
+++ b/include/asm-powerpc/pgtable-ppc32.h
@@ -261,6 +261,7 @@ extern int icache_44x_need_flush;
#define _PAGE_HWEXEC 0x00000004 /* H: Execute permission */
#define _PAGE_ACCESSED 0x00000008 /* S: Page referenced */
#define _PAGE_DIRTY 0x00000010 /* S: Page dirty */
+#define _PAGE_SPECIAL 0x00000020 /* S: Special page */
#define _PAGE_USER 0x00000040 /* S: User page */
#define _PAGE_ENDIAN 0x00000080 /* H: E bit */
#define _PAGE_GUARDED 0x00000100 /* H: G bit */
@@ -276,6 +277,7 @@ extern int icache_44x_need_flush;
/* ERPN in a PTE never gets cleared, ignore it */
#define _PTE_NONE_MASK 0xffffffff00000000ULL
+#define __HAVE_ARCH_PTE_SPECIAL
#elif defined(CONFIG_FSL_BOOKE)
/*
@@ -305,6 +307,7 @@ extern int icache_44x_need_flush;
#define _PAGE_COHERENT 0x00100 /* H: M bit */
#define _PAGE_NO_CACHE 0x00200 /* H: I bit */
#define _PAGE_WRITETHRU 0x00400 /* H: W bit */
+#define _PAGE_SPECIAL 0x00800 /* S: Special page */
#ifdef CONFIG_PTE_64BIT
/* ERPN in a PTE never gets cleared, ignore it */
@@ -315,6 +318,8 @@ extern int icache_44x_need_flush;
#define _PMD_PRESENT_MASK (PAGE_MASK)
#define _PMD_BAD (~PAGE_MASK)
+#define __HAVE_ARCH_PTE_SPECIAL
+
#elif defined(CONFIG_8xx)
/* Definitions for 8xx embedded chips. */
#define _PAGE_PRESENT 0x0001 /* Page is valid */
@@ -362,6 +367,7 @@ extern int icache_44x_need_flush;
#define _PAGE_ACCESSED 0x100 /* R: page referenced */
#define _PAGE_EXEC 0x200 /* software: i-cache coherency required */
#define _PAGE_RW 0x400 /* software: user write access allowed */
+#define _PAGE_SPECIAL 0x800 /* software: Special page */
#define _PTE_NONE_MASK _PAGE_HASHPTE
@@ -372,6 +378,8 @@ extern int icache_44x_need_flush;
/* Hash table based platforms need atomic updates of the linux PTE */
#define PTE_ATOMIC_UPDATES 1
+#define __HAVE_ARCH_PTE_SPECIAL
+
#endif
/*
@@ -404,6 +412,9 @@ extern int icache_44x_need_flush;
#ifndef _PAGE_WRITETHRU
#define _PAGE_WRITETHRU 0
#endif
+#ifndef _PAGE_SPECIAL
+#define _PAGE_SPECIAL 0
+#endif
#ifndef _PMD_PRESENT_MASK
#define _PMD_PRESENT_MASK _PMD_PRESENT
#endif
@@ -533,7 +544,7 @@ static inline int pte_write(pte_t pte) { return pte_val(pte) & _PAGE_RW; }
static inline int pte_dirty(pte_t pte) { return pte_val(pte) & _PAGE_DIRTY; }
static inline int pte_young(pte_t pte) { return pte_val(pte) & _PAGE_ACCESSED; }
static inline int pte_file(pte_t pte) { return pte_val(pte) & _PAGE_FILE; }
-static inline int pte_special(pte_t pte) { return 0; }
+static inline int pte_special(pte_t pte) { return pte_val(pte) & _PAGE_SPECIAL; }
static inline void pte_uncache(pte_t pte) { pte_val(pte) |= _PAGE_NO_CACHE; }
static inline void pte_cache(pte_t pte) { pte_val(pte) &= ~_PAGE_NO_CACHE; }
@@ -552,7 +563,7 @@ static inline pte_t pte_mkdirty(pte_t pte) {
static inline pte_t pte_mkyoung(pte_t pte) {
pte_val(pte) |= _PAGE_ACCESSED; return pte; }
static inline pte_t pte_mkspecial(pte_t pte) {
- return pte; }
+ pte_val(pte) |= _PAGE_SPECIAL; return pte; }
static inline unsigned long pte_pgprot(pte_t pte)
{
return __pgprot(pte_val(pte)) & PAGE_PROT_BITS;
--
1.5.5.1
^ permalink raw reply related
* libata badness
From: Kumar Gala @ 2008-07-31 18:53 UTC (permalink / raw)
To: Jeff Garzik; +Cc: linux-ide, Linux Kernel list, linuxppc-dev list
I'm getting the following badness with top of tree on a embedded
PowerPC w/a ULI 1575 bridge (M5229 IDE):
02:1f.0 IDE interface: Acer Laboratories Inc. [ALi] M5229 IDE (rev c8)
If you need more info let me know.
- k
ahci 0000:02:1f.1: AHCI 0001.0000 32 slots 4 ports 3 Gbps 0xf impl
SATA mode
ahci 0000:02:1f.1: flags: ncq sntf ilck pm led clo pmp pio slum part
------------[ cut here ]------------
Badness at c021e700 [verbose debug info unavailable]
NIP: c021e700 LR: c021e6e8 CTR: c022ce90
REGS: ef82bca0 TRAP: 0700 Not tainted (2.6.27-rc1-00495-g33bd9fe-
dirty)
MSR: 00029032 <EE,ME,IR,DR> CR: 22044022 XER: 20000000
TASK = ef830000[1] 'swapper' THREAD: ef82a000 CPU: 1
GPR00: 00000001 ef82bd50 ef830000 00000000 00009032 00000000 ef0b64fc
00000000
GPR08: ef937c10 c022f93f efbfc8e0 ef900b60 22044028 fffdd3ff 0ffe8700
c044c17c
GPR16: c04d52ec ef82f458 ef82f4f4 ef82f4f4 c044c234 c044c5d8 c044c5d8
c044c220
GPR24: c044c218 c04d52f0 00000080 c022f940 00000000 c044c244 efbec490
00000000
NIP [c021e700] ata_host_activate+0x40/0x10c
LR [c021e6e8] ata_host_activate+0x28/0x10c
Call Trace:
[ef82bd50] [c021e6e8] ata_host_activate+0x28/0x10c (unreliable)
[ef82bd80] [c022f48c] ahci_init_one+0x8b4/0xd68
[ef82be30] [c01b69c0] pci_device_probe+0x84/0xa8
[ef82be50] [c01e5438] driver_probe_device+0xb4/0x1e8
[ef82be70] [c01e55f0] __driver_attach+0x84/0x88
[ef82be90] [c01e4ba0] bus_for_each_dev+0x5c/0xa4
[ef82bec0] [c01e5254] driver_attach+0x24/0x34
[ef82bed0] [c01e44b8] bus_add_driver+0x1d8/0x24c
[ef82bef0] [c01e5810] driver_register+0x70/0x160
[ef82bf10] [c01b6c54] __pci_register_driver+0x64/0xc4
[ef82bf30] [c04aaa60] ahci_init+0x28/0x38
[ef82bf40] [c048717c] do_one_initcall+0x38/0x1ac
[ef82bfb0] [c04874e0] kernel_init+0x1f0/0x1fc
[ef82bff0] [c0013b04] kernel_thread+0x44/0x60
Instruction dump:
7cbb2b78 90010034 7cda3378 7cf93b78 7c7e1b78 4bff9dbd 7c7f1b79 408200c8
2f9c0000 409e002c 313bffff 7c09d910 <0f000000> 80010034 7fc3f378
7f24cb78
scsi0 : ahci
scsi1 : ahci
scsi2 : ahci
scsi3 : ahci
ata1: SATA max UDMA/133 abar m1024@0x80006000 port 0x80006100
ata2: SATA max UDMA/133 abar m1024@0x80006000 port 0x80006180
ata3: SATA max UDMA/133 abar m1024@0x80006000 port 0x80006200
ata4: SATA max UDMA/133 abar m1024@0x80006000 port 0x80006280
ata1: SATA link down (SStatus 0 SControl 300)
ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
ata2.00: qc timeout (cmd 0xec)
ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
ata2.00: qc timeout (cmd 0xec)
ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
ata2.00: qc timeout (cmd 0xec)
ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
ata3: SATA link down (SStatus 0 SControl 300)
ata4: SATA link down (SStatus 0 SControl 300)
scsi4 : pata_ali
scsi5 : pata_ali
ata5: PATA max UDMA/133 cmd 0x1200 ctl 0x1208 bmdma 0x1220
ata6: PATA max UDMA/133 cmd 0x1210 ctl 0x1218 bmdma 0x1228
ata5.00: ATAPI: TSSTcorp CDW/DVD SH-M522C, TS06, max UDMA/33
ata5.00: WARNING: ATAPI DMA disabled for reliablity issues. It can be
enabled
ata5.00: WARNING: via pata_ali.atapi_dma modparam or corresponding
sysfs node.
ata5.00: configured for UDMA/33
ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
ata5.00: status: { DRDY }
ata5: soft resetting link
ata5.00: configured for UDMA/33
ata5: EH complete
ata5.00: limiting speed to UDMA/25:PIO4
ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
ata5.00: status: { DRDY }
ata5: soft resetting link
ata5.00: configured for UDMA/25
ata5: EH complete
ata5.00: limiting speed to PIO4
ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
ata5.00: status: { DRDY }
ata5: soft resetting link
ata5.00: configured for PIO4
ata5: EH complete
ata5.00: limiting speed to PIO3
ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
ata5.00: status: { DRDY }
ata5: soft resetting link
ata5.00: configured for PIO3
ata5: EH complete
ata5.00: limiting speed to PIO0
ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
ata5.00: status: { DRDY }
ata5: soft resetting link
ata5.00: configured for PIO0
ata5: EH complete
ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
ata5.00: status: { DRDY }
ata5: soft resetting link
ata5.00: configured for PIO0
ata5: EH complete
ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
ata5.00: status: { DRDY }
ata5: soft resetting link
ata5.00: configured for PIO0
ata5: EH complete
ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
ata5.00: status: { DRDY }
ata5: soft resetting link
ata5.00: configured for PIO0
ata5: EH complete
ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
ata5.00: status: { DRDY }
ata5: soft resetting link
ata5.00: configured for PIO0
ata5: EH complete
ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
ata5.00: status: { DRDY }
ata5: soft resetting link
ata5.00: configured for PIO0
ata5: EH complete
ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
ata5.00: status: { DRDY }
ata5: soft resetting link
ata5.00: configured for PIO0
ata5: EH complete
ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
ata5.00: status: { DRDY }
ata5: soft resetting link
ata5.00: configured for PIO0
ata5: EH complete
ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
ata5.00: status: { DRDY }
ata5: soft resetting link
ata5.00: configured for PIO0
ata5: EH complete
ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
ata5.00: status: { DRDY }
ata5: soft resetting link
ata5.00: configured for PIO0
ata5: EH complete
ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
ata5.00: status: { DRDY }
ata5: soft resetting link
ata5.00: configured for PIO0
ata5: EH complete
ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
ata5.00: status: { DRDY }
ata5: soft resetting link
ata5.00: configured for PIO0
ata5: EH complete
ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
ata5.00: status: { DRDY }
ata5: soft resetting link
ata5.00: configured for PIO0
ata5: EH complete
ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
ata5.00: status: { DRDY }
ata5: soft resetting link
ata5.00: configured for PIO0
ata5: EH complete
ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
ata5.00: status: { DRDY }
ata5: soft resetting link
ata5.00: configured for PIO0
ata5: EH complete
ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
ata5.00: status: { DRDY }
ata5: soft resetting link
ata5.00: configured for PIO0
ata5: EH complete
ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
ata5.00: status: { DRDY }
ata5: soft resetting link
ata5.00: configured for PIO0
ata5: EH complete
ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
ata5.00: status: { DRDY }
ata5: soft resetting link
ata5.00: configured for PIO0
ata5: EH complete
ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
ata5.00: status: { DRDY }
ata5: soft resetting link
ata5.00: configured for PIO0
ata5: EH complete
ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
ata5.00: status: { DRDY }
ata5: soft resetting link
ata5.00: configured for PIO0
ata5: EH complete
ata5: WARNING: synchronous SCSI scan failed without making any progress,
switching to async
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Jon Smirl @ 2008-07-31 18:57 UTC (permalink / raw)
To: Timur Tabi; +Cc: Scott Wood, Linux I2C, Linuxppc-dev
In-Reply-To: <48920607.5040606@freescale.com>
On 7/31/08, Timur Tabi <timur@freescale.com> wrote:
> Grant Likely wrote:
>
> > No it doesn't, it depends on the register interface to decide
> > compatibility. Clock interface is part of that.
>
>
> I don't think so. The interface for programming the clock registers is
> identical on all 8[356]xx parts. The only thing that matters is what specific
> values to put in the FDR and DFSR registers to get a desired I2C bus speed.
> That answer is dependent on the actual clock input to the device, which is
> external to the device. I wouldn't call the input frequency a property of the
> I2C device.
>
>
> > I suggested encoding
> > the clock divider directly in compatible (implicit in the SoC version),
> > but it doesn't have to be that way. If clock freq is obtained from
> > another property or some other method then that is okay too.
>
>
> I think we agree on that.
>
>
> > There is nothing wrong with it (as long as we agree and it gets
> > documented). I certainly don't have a problem with doing it this way.
>
>
> I propose the property "clock-frequency", like this:
>
> i2c@3000 {
> #address-cells = <1>;
> #size-cells = <0>;
> cell-index = <0>;
> compatible = "fsl-i2c";
> reg = <0x3000 0x100>;
> interrupts = <14 0x8>;
> interrupt-parent = <&ipic>;
> dfsrr;
The existence of the dfsrr and fsl,mpc5200-i2c attributes imply to me
that the compatible strings were not done correctly. If these devices
really were compatible we wouldn't need extra attributes to tell them
apart.
So I'm with Grant on adding compatible strings sufficient to remove
the need for dfsrr and fsl,mpc5200-i2c.
Something like...
static const struct of_device_id mpc_i2c_of_match[] = {
{.compatible = "fsl,mpc5200b-i2c", .data = fsl_i2c_mpc5200, },
{.compatible = "fsl,mpc5200-i2c", .data = fsl_i2c_mpc5200, },
{.compatible = "fsl,mpc8xxx-i2c", .data = fsl_i2c_dfsrr, },
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 mpc8xxxx 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.
I'd like to move towards a more generic uboot that gets the processor
minimally running and then use the device tree to customize as many
things as possible. But that's just my opinion.
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Timur Tabi @ 2008-07-31 19:01 UTC (permalink / raw)
To: Jon Smirl; +Cc: Scott Wood, Linux I2C, Linuxppc-dev
In-Reply-To: <9e4733910807311157q358640ddyef1f14865c069b8@mail.gmail.com>
Jon Smirl wrote:
> The existence of the dfsrr and fsl,mpc5200-i2c attributes imply to me
> that the compatible strings were not done correctly. If these devices
> really were compatible we wouldn't need extra attributes to tell them
> apart.
I agree with that.
> So I'm with Grant on adding compatible strings sufficient to remove
> the need for dfsrr and fsl,mpc5200-i2c.
Let's just make sure we don't break backwards compatibility.
> Something like...
> static const struct of_device_id mpc_i2c_of_match[] = {
> {.compatible = "fsl,mpc5200b-i2c", .data = fsl_i2c_mpc5200, },
> {.compatible = "fsl,mpc5200-i2c", .data = fsl_i2c_mpc5200, },
> {.compatible = "fsl,mpc8xxx-i2c", .data = fsl_i2c_dfsrr, },
That seems ok.
> 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 mpc8xxxx 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.
I don't want to differentiate between 52xx and 8xxx any more than we have to.
If 8xxx has the clock frequency in the device tree, then 52xx should have it
there, too.
For backwards compatibility purposes, we can have the driver provide a
hard-coded value of some kind if the property does not exist.
> I'd like to move towards a more generic uboot that gets the processor
> minimally running and then use the device tree to customize as many
> things as possible. But that's just my opinion.
U-Boot needs to configure the I2C bus speed because U-Boot uses the I2C. We
should capitalize on that by taking the information that U-Boot has calculated
and re-use it.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Scott Wood @ 2008-07-31 19:01 UTC (permalink / raw)
To: Timur Tabi; +Cc: Linuxppc-dev, Linux I2C
In-Reply-To: <48920607.5040606@freescale.com>
Timur Tabi wrote:
> Grant Likely wrote:
>
>> No it doesn't, it depends on the register interface to decide
>> compatibility. Clock interface is part of that.
>
> I don't think so. The interface for programming the clock registers is
> identical on all 8[356]xx parts. The only thing that matters is what specific
> values to put in the FDR and DFSR registers to get a desired I2C bus speed.
If it affects the values you need to write to the registers to achieve a
given result, how is it not a difference in the register interface?
> I propose the property "clock-frequency", like this:
>
> i2c@3000 {
> #address-cells = <1>;
> #size-cells = <0>;
> cell-index = <0>;
> compatible = "fsl-i2c";
> reg = <0x3000 0x100>;
> interrupts = <14 0x8>;
> interrupt-parent = <&ipic>;
> dfsrr;
> clock-frequency = <0xblablabla>; <-- added by U-Boot
> };
A clock-frequency property is OK, and is in line with what we do in
other types of nodes. However, in the long run it might be nice to
introduce some sort of clock binding where, for example, the i2c node
can point to a clock elsewhere in the device tree as an input clock.
That way, less knowledge is required by the firmware to poke values all
over the place, and it also allows one to describe situations where the
frequency of the input clock can change (such as in low-power modes).
-Scott
^ permalink raw reply
* Re: [PATCH] powerpc/mm: Implement _PAGE_SPECIAL & pte_special() for 32-bit
From: Kumar Gala @ 2008-07-31 18:50 UTC (permalink / raw)
To: Josh Boyer; +Cc: Nick Piggin, linuxppc-dev list, Linux Kernel list
In-Reply-To: <Pine.LNX.4.64.0807311345380.8012@blarg.am.freescale.net>
On Jul 31, 2008, at 1:48 PM, Kumar Gala wrote:
> Implement _PAGE_SPECIAL and pte_special() for 32-bit powerpc. This
> bit will
> be used by the fast get_user_pages() to differenciate PTEs that
> correspond
> to a valid struct page from special mappings that don't such as IO
> mappings
> obtained via io_remap_pfn_ranges().
>
> We currently only implement this on sub-arch that support SMP or
> will so
> in the future (6xx, 44x, FSL-BookE) and not (8xx, 40x).
>
> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
> ---
> Nick, do you forsee using _PAGE_SPECIAL for other applications that
> would
> be of interested to non-SMP hw?
>
> We can look at adding it into 8xx and 40x, but was being lazy as I
> assume
> there is no point.
>
> - k
Josh,
Can you test this on 44x for me.
thanks
- k
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Timur Tabi @ 2008-07-31 19:08 UTC (permalink / raw)
To: Scott Wood; +Cc: Linuxppc-dev, Linux I2C
In-Reply-To: <48920C1A.7010605@freescale.com>
Scott Wood wrote:
> Timur Tabi wrote:
>> Grant Likely wrote:
>>
>>> No it doesn't, it depends on the register interface to decide
>>> compatibility. Clock interface is part of that.
>> I don't think so. The interface for programming the clock registers is
>> identical on all 8[356]xx parts. The only thing that matters is what specific
>> values to put in the FDR and DFSR registers to get a desired I2C bus speed.
>
> If it affects the values you need to write to the registers to achieve a
> given result, how is it not a difference in the register interface?
I think we're splitting hairs, here. The code for actually programming the
registers is the same, regardless of the input frequency. It's just that the
input frequency is a value needed to calculate the right value. But like I
said, I don't think this distinction is that relevant.
> A clock-frequency property is OK, and is in line with what we do in
> other types of nodes. However, in the long run it might be nice to
> introduce some sort of clock binding where, for example, the i2c node
> can point to a clock elsewhere in the device tree as an input clock.
The only problem with that is that the actual input clock to the I2C device is
not the same as any other device. It's a unique clock. Look at the code I had
to write to figure out this clock just on 85xx:
/*
* The base clock for I2C depends on the actual SOC. Unfortunately,
* there is no pattern that can be used to determine the frequency, so
* the only choice is to look up the actual SOC number and use the value
* for that SOC. This information is taken from application note
* AN2919.
*/
#if defined(CONFIG_MPC8540) || defined(CONFIG_MPC8541) || \
defined(CONFIG_MPC8560) || defined(CONFIG_MPC8555)
gd->i2c1_clk = sys_info.freqSystemBus;
#elif defined(CONFIG_MPC8544)
/*
* 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
This is just for 85xx, and it only applies to the I2C devices.
> That way, less knowledge is required by the firmware to poke values all
> over the place, and it also allows one to describe situations where the
> frequency of the input clock can change (such as in low-power modes).
I don't think it's possible to do what you want to do. I2C clocking is
completely screwed up, and that's just the way the hardware was designed.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Jon Smirl @ 2008-07-31 19:11 UTC (permalink / raw)
To: Scott Wood; +Cc: Linuxppc-dev, Timur Tabi, Linux I2C
In-Reply-To: <48920C1A.7010605@freescale.com>
On 7/31/08, Scott Wood <scottwood@freescale.com> wrote:
> Timur Tabi wrote:
>
> > Grant Likely wrote:
> >
> >
> > > No it doesn't, it depends on the register interface to decide
> > > compatibility. Clock interface is part of that.
> > >
> >
> > I don't think so. The interface for programming the clock registers is
> > identical on all 8[356]xx parts. The only thing that matters is what
> specific
> > values to put in the FDR and DFSR registers to get a desired I2C bus
> speed.
> >
>
> If it affects the values you need to write to the registers to achieve a
> given result, how is it not a difference in the register interface?
>
>
> > I propose the property "clock-frequency", like this:
> >
> > i2c@3000 {
> > #address-cells = <1>;
> > #size-cells = <0>;
> > cell-index = <0>;
> > compatible = "fsl-i2c";
> > reg = <0x3000 0x100>;
> > interrupts = <14 0x8>;
> > interrupt-parent = <&ipic>;
> > dfsrr;
> > clock-frequency = <0xblablabla>; <-- added by
> U-Boot
> > };
> >
>
> A clock-frequency property is OK, and is in line with what we do in other
> types of nodes. However, in the long run it might be nice to introduce some
> sort of clock binding where, for example, the i2c node can point to a clock
> elsewhere in the device tree as an input clock.
>
> That way, less knowledge is required by the firmware to poke values all
> over the place, and it also allows one to describe situations where the
> frequency of the input clock can change (such as in low-power modes).
PowerPC,5200@0 {
timebase-frequency = <0>; /* From Bootloader */
bus-frequency = <0>; /* From Bootloader */
clock-frequency = <0>; /* From Bootloader */
};
The mpc5200 code already has mpc52xx_find_ipb_freq() to get it.
I should grep before suggesting things.
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Grant Likely @ 2008-07-31 19:14 UTC (permalink / raw)
To: Timur Tabi; +Cc: Scott Wood, Linuxppc-dev, Linux I2C
In-Reply-To: <48920607.5040606@freescale.com>
On Thu, Jul 31, 2008 at 01:35:51PM -0500, Timur Tabi wrote:
> Grant Likely wrote:
>
> > No it doesn't, it depends on the register interface to decide
> > compatibility. Clock interface is part of that.
>
> I don't think so. The interface for programming the clock registers is
> identical on all 8[356]xx parts. The only thing that matters is what specific
> values to put in the FDR and DFSR registers to get a desired I2C bus speed.
> That answer is dependent on the actual clock input to the device, which is
> external to the device. I wouldn't call the input frequency a property of the
> I2C device.
Okay, I accept that argument. Make input frequency a property and be
done with it.
> > I suggested encoding
> > the clock divider directly in compatible (implicit in the SoC version),
> > but it doesn't have to be that way. If clock freq is obtained from
> > another property or some other method then that is okay too.
>
> I think we agree on that.
indeed.
> > There is nothing wrong with it (as long as we agree and it gets
> > documented). I certainly don't have a problem with doing it this way.
>
> I propose the property "clock-frequency", like this:
>
> i2c@3000 {
> #address-cells = <1>;
> #size-cells = <0>;
> cell-index = <0>;
> compatible = "fsl-i2c";
> reg = <0x3000 0x100>;
> interrupts = <14 0x8>;
> interrupt-parent = <&ipic>;
> dfsrr;
> clock-frequency = <0xblablabla>; <-- added by U-Boot
> };
I'm okay with this.
> Note that the dfsrr property already differentiates between 8xxx and 52xx, so
> maybe we don't need any other device tree changes.
The whole dfsrr property is a really bad idea, and it just replaces the
equally bad idea of an "mpc5200-clocking" property which is currently in
use. This bit should definitely be determined via compatible.
g.
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Scott Wood @ 2008-07-31 19:15 UTC (permalink / raw)
To: Timur Tabi; +Cc: Linuxppc-dev, Linux I2C
In-Reply-To: <48920D9B.5060406@freescale.com>
Timur Tabi wrote:
> Scott Wood wrote:
>> A clock-frequency property is OK, and is in line with what we do in
>> other types of nodes. However, in the long run it might be nice to
>> introduce some sort of clock binding where, for example, the i2c node
>> can point to a clock elsewhere in the device tree as an input clock.
>
> The only problem with that is that the actual input clock to the I2C device is
> not the same as any other device. It's a unique clock. Look at the code I had
> to write to figure out this clock just on 85xx:
IIRC, only the divider is unique, and the divider that is applied to the
input clock can be specified in the i2c node (either implicitly in
compatible, or explicitly via a property).
-Scott
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Timur Tabi @ 2008-07-31 19:19 UTC (permalink / raw)
To: Scott Wood; +Cc: Linuxppc-dev, Linux I2C
In-Reply-To: <48920F69.6020909@freescale.com>
Scott Wood wrote:
> Timur Tabi wrote:
>> Scott Wood wrote:
>>> A clock-frequency property is OK, and is in line with what we do in
>>> other types of nodes. However, in the long run it might be nice to
>>> introduce some sort of clock binding where, for example, the i2c node
>>> can point to a clock elsewhere in the device tree as an input clock.
>> The only problem with that is that the actual input clock to the I2C device is
>> not the same as any other device. It's a unique clock. Look at the code I had
>> to write to figure out this clock just on 85xx:
>
> IIRC, only the divider is unique, and the divider that is applied to the
> input clock can be specified in the i2c node (either implicitly in
> compatible, or explicitly via a property).
True, but I'd rather we have a real clock-frequency property that contains the
calculated I2C input frequency, than a divider. It's more consistent with other
properties, and it hides the complicated nature of I2C clocking.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Scott Wood @ 2008-07-31 19:21 UTC (permalink / raw)
To: Timur Tabi; +Cc: Linuxppc-dev, Linux I2C
In-Reply-To: <48921057.8030807@freescale.com>
Timur Tabi wrote:
> True, but I'd rather we have a real clock-frequency property that contains the
> calculated I2C input frequency, than a divider. It's more consistent with other
> properties,
Right, I wasn't saying i2c should be different from serial, pci, etc.
It was more of a side comment about the way we specify clocks in general.
-Scott
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Timur Tabi @ 2008-07-31 19:22 UTC (permalink / raw)
To: Scott Wood; +Cc: Linuxppc-dev, Linux I2C
In-Reply-To: <489210AF.7000909@freescale.com>
Scott Wood wrote:
> Timur Tabi wrote:
>> True, but I'd rather we have a real clock-frequency property that contains the
>> calculated I2C input frequency, than a divider. It's more consistent with other
>> properties,
>
> Right, I wasn't saying i2c should be different from serial, pci, etc.
> It was more of a side comment about the way we specify clocks in general.
I guess it's agreed then. If no one else gets around to it, I'll submit the
U-Boot patch for updating the device tree.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Timur Tabi @ 2008-07-31 19:24 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: Scott Wood, Linux I2C, Linuxppc-dev
In-Reply-To: <48921187.1090101@grandegger.com>
Wolfgang Grandegger wrote:
> No, the source clock is not identical for all 8[356]xx. Some use half or
> even a third of the SOC clock frequency.
The platform clock divided by 2 or 3 *is* the source clock to the I2C. That's
what I'm talking about.
> Linux must determine the real
> source clock frequency somehow. We may introduce the SOC property
> "i2c-clock-frequency", which could be fixed up by U-Boot or a pre-loader
> (in case U-Boot is not used). Like for other frequency properties as well.
That's what I'm proposing, but drop the "i2c-".
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Wolfgang Grandegger @ 2008-07-31 19:24 UTC (permalink / raw)
To: Timur Tabi; +Cc: Scott Wood, Linux I2C, Linuxppc-dev
In-Reply-To: <48920607.5040606@freescale.com>
Timur Tabi wrote:
> Grant Likely wrote:
>
>> No it doesn't, it depends on the register interface to decide
>> compatibility. Clock interface is part of that.
>
> I don't think so. The interface for programming the clock registers is
> identical on all 8[356]xx parts. The only thing that matters is what specific
> values to put in the FDR and DFSR registers to get a desired I2C bus speed.
> That answer is dependent on the actual clock input to the device, which is
> external to the device. I wouldn't call the input frequency a property of the
> I2C device.
No, the source clock is not identical for all 8[356]xx. Some use half or
even a third of the SOC clock frequency. Linux must determine the real
source clock frequency somehow. We may introduce the SOC property
"i2c-clock-frequency", which could be fixed up by U-Boot or a pre-loader
(in case U-Boot is not used). Like for other frequency properties as well.
>> I suggested encoding
>> the clock divider directly in compatible (implicit in the SoC version),
>> but it doesn't have to be that way. If clock freq is obtained from
>> another property or some other method then that is okay too.
>
> I think we agree on that.
>
>> There is nothing wrong with it (as long as we agree and it gets
>> documented). I certainly don't have a problem with doing it this way.
>
> I propose the property "clock-frequency", like this:
>
> i2c@3000 {
> #address-cells = <1>;
> #size-cells = <0>;
> cell-index = <0>;
> compatible = "fsl-i2c";
> reg = <0x3000 0x100>;
> interrupts = <14 0x8>;
> interrupt-parent = <&ipic>;
> dfsrr;
> clock-frequency = <0xblablabla>; <-- added by U-Boot
> };
I think we already agreed to use the property "clock-frequency" for the
real I2C clock frequency. If it is not provided, the old settings, e.g.
from U-Boot, will be kept.
> Note that the dfsrr property already differentiates between 8xxx and 52xx, so
> maybe we don't need any other device tree changes.
It should be done via the compatible property.
Wolfgang.
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Grant Likely @ 2008-07-31 19:25 UTC (permalink / raw)
To: Jon Smirl; +Cc: Scott Wood, Linuxppc-dev, Timur Tabi, Linux I2C
In-Reply-To: <9e4733910807311157q358640ddyef1f14865c069b8@mail.gmail.com>
On Thu, Jul 31, 2008 at 02:57:25PM -0400, Jon Smirl wrote:
> On 7/31/08, Timur Tabi <timur@freescale.com> wrote:
> > Grant Likely wrote:
> > I propose the property "clock-frequency", like this:
> >
> > i2c@3000 {
> > #address-cells = <1>;
> > #size-cells = <0>;
> > cell-index = <0>;
> > compatible = "fsl-i2c";
> > reg = <0x3000 0x100>;
> > interrupts = <14 0x8>;
> > interrupt-parent = <&ipic>;
> > dfsrr;
>
> The existence of the dfsrr and fsl,mpc5200-i2c attributes imply to me
> that the compatible strings were not done correctly. If these devices
> really were compatible we wouldn't need extra attributes to tell them
> apart.
Yes, exactly right.
> So I'm with Grant on adding compatible strings sufficient to remove
> the need for dfsrr and fsl,mpc5200-i2c.
>
> Something like...
> static const struct of_device_id mpc_i2c_of_match[] = {
> {.compatible = "fsl,mpc5200b-i2c", .data = fsl_i2c_mpc5200, },
> {.compatible = "fsl,mpc5200-i2c", .data = fsl_i2c_mpc5200, },
> {.compatible = "fsl,mpc8xxx-i2c", .data = fsl_i2c_dfsrr, },
Yes, but I don't agree with the last entry in this list.
"fsl,mpc8xxx-i2c" is not a good value. Use an actual part number
instead and have newer devices claim compatibility with the old.
The problem with 8xxx is that you don't know if/when another 8xxx
will arrive on the scene which does not conform to already made
assumptions. If you specify an exact SoC part, then the problem goes
away without any downside.
> 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 mpc8xxxx 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.
In general, I don't like adding additional global or machine vars to
Linux. I just don't think it in necessary and it can quickly get out of
control. Instead, For 5200 I've exported a mpc52xx specific function
that returns the clock frequency. The function can be adapted over time
to handle new cases on the 52xx platform and it get compiled out if 5200
is not in use.
As an alternative, clock-frequency could be made an optional property.
If it isn't specified, then get the bus-frequency property from the
parent node instead.
g.
^ permalink raw reply
* Re: libata badness
From: Ben Dooks @ 2008-07-31 19:02 UTC (permalink / raw)
To: Kumar Gala; +Cc: linux-ide, Jeff Garzik, Linux Kernel list, linuxppc-dev list
In-Reply-To: <95646AC3-5385-4F59-8EBA-20F811168A87@kernel.crashing.org>
On Thu, Jul 31, 2008 at 01:53:45PM -0500, Kumar Gala wrote:
> I'm getting the following badness with top of tree on a embedded
> PowerPC w/a ULI 1575 bridge (M5229 IDE):
>
> 02:1f.0 IDE interface: Acer Laboratories Inc. [ALi] M5229 IDE (rev c8)
>
> If you need more info let me know.
>
> - k
>
> ahci 0000:02:1f.1: AHCI 0001.0000 32 slots 4 ports 3 Gbps 0xf impl
> SATA mode
> ahci 0000:02:1f.1: flags: ncq sntf ilck pm led clo pmp pio slum part
> ------------[ cut here ]------------
> Badness at c021e700 [verbose debug info unavailable]
it would be helpful to compile a kernel with verbose fault information
turned on.
> NIP: c021e700 LR: c021e6e8 CTR: c022ce90
> REGS: ef82bca0 TRAP: 0700 Not tainted (2.6.27-rc1-00495-g33bd9fe-
> dirty)
> MSR: 00029032 <EE,ME,IR,DR> CR: 22044022 XER: 20000000
> TASK = ef830000[1] 'swapper' THREAD: ef82a000 CPU: 1
> GPR00: 00000001 ef82bd50 ef830000 00000000 00009032 00000000 ef0b64fc
> 00000000
> GPR08: ef937c10 c022f93f efbfc8e0 ef900b60 22044028 fffdd3ff 0ffe8700
> c044c17c
> GPR16: c04d52ec ef82f458 ef82f4f4 ef82f4f4 c044c234 c044c5d8 c044c5d8
> c044c220
> GPR24: c044c218 c04d52f0 00000080 c022f940 00000000 c044c244 efbec490
> 00000000
> NIP [c021e700] ata_host_activate+0x40/0x10c
> LR [c021e6e8] ata_host_activate+0x28/0x10c
> Call Trace:
> [ef82bd50] [c021e6e8] ata_host_activate+0x28/0x10c (unreliable)
> [ef82bd80] [c022f48c] ahci_init_one+0x8b4/0xd68
> [ef82be30] [c01b69c0] pci_device_probe+0x84/0xa8
> [ef82be50] [c01e5438] driver_probe_device+0xb4/0x1e8
> [ef82be70] [c01e55f0] __driver_attach+0x84/0x88
> [ef82be90] [c01e4ba0] bus_for_each_dev+0x5c/0xa4
> [ef82bec0] [c01e5254] driver_attach+0x24/0x34
> [ef82bed0] [c01e44b8] bus_add_driver+0x1d8/0x24c
> [ef82bef0] [c01e5810] driver_register+0x70/0x160
> [ef82bf10] [c01b6c54] __pci_register_driver+0x64/0xc4
> [ef82bf30] [c04aaa60] ahci_init+0x28/0x38
> [ef82bf40] [c048717c] do_one_initcall+0x38/0x1ac
> [ef82bfb0] [c04874e0] kernel_init+0x1f0/0x1fc
> [ef82bff0] [c0013b04] kernel_thread+0x44/0x60
> Instruction dump:
> 7cbb2b78 90010034 7cda3378 7cf93b78 7c7e1b78 4bff9dbd 7c7f1b79 408200c8
> 2f9c0000 409e002c 313bffff 7c09d910 <0f000000> 80010034 7fc3f378
> 7f24cb78
> scsi0 : ahci
> scsi1 : ahci
> scsi2 : ahci
> scsi3 : ahci
> ata1: SATA max UDMA/133 abar m1024@0x80006000 port 0x80006100
> ata2: SATA max UDMA/133 abar m1024@0x80006000 port 0x80006180
> ata3: SATA max UDMA/133 abar m1024@0x80006000 port 0x80006200
> ata4: SATA max UDMA/133 abar m1024@0x80006000 port 0x80006280
> ata1: SATA link down (SStatus 0 SControl 300)
> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
> ata2.00: qc timeout (cmd 0xec)
> ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
> ata2.00: qc timeout (cmd 0xec)
> ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
> ata2.00: qc timeout (cmd 0xec)
> ata2.00: failed to IDENTIFY (I/O error, err_mask=0x4)
> ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
> ata3: SATA link down (SStatus 0 SControl 300)
> ata4: SATA link down (SStatus 0 SControl 300)
> scsi4 : pata_ali
> scsi5 : pata_ali
> ata5: PATA max UDMA/133 cmd 0x1200 ctl 0x1208 bmdma 0x1220
> ata6: PATA max UDMA/133 cmd 0x1210 ctl 0x1218 bmdma 0x1228
Looks like you're running into the same problem as I did, with the
fact that the M5529 is in native mode, but doesn't have any IRQ
available. Do you know if the bridge chip in it is in routes the
IRQs internally?
> ata5.00: ATAPI: TSSTcorp CDW/DVD SH-M522C, TS06, max UDMA/33
> ata5.00: WARNING: ATAPI DMA disabled for reliablity issues. It can be
> enabled
> ata5.00: WARNING: via pata_ali.atapi_dma modparam or corresponding
> sysfs node.
> ata5.00: configured for UDMA/33
> ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
> cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
> res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
> ata5.00: status: { DRDY }
> ata5: soft resetting link
> ata5.00: configured for UDMA/33
> ata5: EH complete
> ata5.00: limiting speed to UDMA/25:PIO4
> ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
> cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
> res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
> ata5.00: status: { DRDY }
> ata5: soft resetting link
> ata5.00: configured for UDMA/25
> ata5: EH complete
> ata5.00: limiting speed to PIO4
> ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
> cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
> res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
> ata5.00: status: { DRDY }
> ata5: soft resetting link
> ata5.00: configured for PIO4
> ata5: EH complete
> ata5.00: limiting speed to PIO3
> ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
> cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
> res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
> ata5.00: status: { DRDY }
> ata5: soft resetting link
> ata5.00: configured for PIO3
> ata5: EH complete
> ata5.00: limiting speed to PIO0
> ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
> cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
> res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
> ata5.00: status: { DRDY }
> ata5: soft resetting link
> ata5.00: configured for PIO0
> ata5: EH complete
> ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
> cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
> res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
> ata5.00: status: { DRDY }
> ata5: soft resetting link
> ata5.00: configured for PIO0
> ata5: EH complete
> ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
> cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
> res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
> ata5.00: status: { DRDY }
> ata5: soft resetting link
> ata5.00: configured for PIO0
> ata5: EH complete
> ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
> cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
> res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
> ata5.00: status: { DRDY }
> ata5: soft resetting link
> ata5.00: configured for PIO0
> ata5: EH complete
> ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
> cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
> res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
> ata5.00: status: { DRDY }
> ata5: soft resetting link
> ata5.00: configured for PIO0
> ata5: EH complete
> ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
> cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
> res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
> ata5.00: status: { DRDY }
> ata5: soft resetting link
> ata5.00: configured for PIO0
> ata5: EH complete
> ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
> cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
> res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
> ata5.00: status: { DRDY }
> ata5: soft resetting link
> ata5.00: configured for PIO0
> ata5: EH complete
> ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
> cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
> res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
> ata5.00: status: { DRDY }
> ata5: soft resetting link
> ata5.00: configured for PIO0
> ata5: EH complete
> ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
> cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
> res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
> ata5.00: status: { DRDY }
> ata5: soft resetting link
> ata5.00: configured for PIO0
> ata5: EH complete
> ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
> cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
> res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
> ata5.00: status: { DRDY }
> ata5: soft resetting link
> ata5.00: configured for PIO0
> ata5: EH complete
> ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
> cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
> res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
> ata5.00: status: { DRDY }
> ata5: soft resetting link
> ata5.00: configured for PIO0
> ata5: EH complete
> ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
> cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
> res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
> ata5.00: status: { DRDY }
> ata5: soft resetting link
> ata5.00: configured for PIO0
> ata5: EH complete
> ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
> cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
> res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
> ata5.00: status: { DRDY }
> ata5: soft resetting link
> ata5.00: configured for PIO0
> ata5: EH complete
> ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
> cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
> res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
> ata5.00: status: { DRDY }
> ata5: soft resetting link
> ata5.00: configured for PIO0
> ata5: EH complete
> ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
> cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
> res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
> ata5.00: status: { DRDY }
> ata5: soft resetting link
> ata5.00: configured for PIO0
> ata5: EH complete
> ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
> cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
> res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
> ata5.00: status: { DRDY }
> ata5: soft resetting link
> ata5.00: configured for PIO0
> ata5: EH complete
> ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
> cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
> res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
> ata5.00: status: { DRDY }
> ata5: soft resetting link
> ata5.00: configured for PIO0
> ata5: EH complete
> ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
> cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
> res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
> ata5.00: status: { DRDY }
> ata5: soft resetting link
> ata5.00: configured for PIO0
> ata5: EH complete
> ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
> cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
> res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
> ata5.00: status: { DRDY }
> ata5: soft resetting link
> ata5.00: configured for PIO0
> ata5: EH complete
> ata5.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> ata5.00: cmd a0/00:00:00:24:00/00:00:00:00:00/a0 tag 0 pio 36 in
> cdb 12 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00
> res 40/00:00:00:00:00/00:00:00:00:00/00 Emask 0x4 (timeout)
> ata5.00: status: { DRDY }
> ata5: soft resetting link
> ata5.00: configured for PIO0
> ata5: EH complete
> ata5: WARNING: synchronous SCSI scan failed without making any progress,
> switching to async
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ide" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
--
Ben
Q: What's a light-year?
A: One-third less calories than a regular year.
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Wolfgang Grandegger @ 2008-07-31 19:54 UTC (permalink / raw)
To: Timur Tabi; +Cc: Scott Wood, Linux I2C, Linuxppc-dev
In-Reply-To: <48921179.1080403@freescale.com>
Timur Tabi wrote:
> Wolfgang Grandegger wrote:
>
>> No, the source clock is not identical for all 8[356]xx. Some use half or
>> even a third of the SOC clock frequency.
>
> The platform clock divided by 2 or 3 *is* the source clock to the I2C. That's
> what I'm talking about.
>
>> Linux must determine the real
>> source clock frequency somehow. We may introduce the SOC property
>> "i2c-clock-frequency", which could be fixed up by U-Boot or a pre-loader
>> (in case U-Boot is not used). Like for other frequency properties as well.
>
> That's what I'm proposing, but drop the "i2c-".
But clock-frequency, aka bus-frequency, is already used by
fsl_get_sys_freq():
http://lxr.linux.no/linux+v2.6.26/arch/powerpc/sysdev/fsl_soc.c#L80
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:
soc8541@e0000000 {
#address-cells = <1>;
#size-cells = <1>;
device_type = "soc";
ranges = <0x0 0xe0000000 0x100000>;
reg = <0xe0000000 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().
Wolfgang.
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Timur Tabi @ 2008-07-31 19:58 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: Scott Wood, Linux I2C, Linuxppc-dev
In-Reply-To: <48921888.3020900@grandegger.com>
Wolfgang Grandegger wrote:
> But clock-frequency, aka bus-frequency, is already used by
> fsl_get_sys_freq():
>
> http://lxr.linux.no/linux+v2.6.26/arch/powerpc/sysdev/fsl_soc.c#L80
So? clock-frequency is a per-node property. I use it in the codec node on the
8610 (mpc8610_hpcd.dts). It does not mean "platform clock frequency".
> U-Boot could then fixup that value like bus-frequency() and the i2c-mpc
> driver simply calls fsl_get_i2c_freq().
This is just more complicated than it needs to be. Why should the I2C driver
fetch the platform clock and the divider from the parent node, and then do
additional math, when it could just get the value it needs right from the node
it's probing?
Besides, U-Boot does not currently store the divider value. Look at the code
I've posted twice already - it stores the frequency in i2c1_clk. So now I would
need to create another variable in the gd_t to store the divider? No thanks.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Grant Likely @ 2008-07-31 19:59 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: Scott Wood, Linuxppc-dev, Timur Tabi, Linux I2C
In-Reply-To: <48921888.3020900@grandegger.com>
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:
>
> soc8541@e0000000 {
> #address-cells = <1>;
> #size-cells = <1>;
> device_type = "soc";
> ranges = <0x0 0xe0000000 0x100000>;
> reg = <0xe0000000 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().
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.
g.
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Timur Tabi @ 2008-07-31 20:00 UTC (permalink / raw)
To: Grant Likely; +Cc: Scott Wood, Linuxppc-dev, Linux I2C
In-Reply-To: <20080731195911.GA29610@secretlab.ca>
Grant Likely wrote:
> 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.
His proposal would make the implementation less simple, not more simple.
> 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.
It isn't. I don't have the Verilog for the I2C device, but I'm pretty sure the
divider is a very simple circuit that's located just outside the I2C circuitry,
but it still unique to the I2C device. There is no "I2C clock" running through
the SOC.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Wolfgang Grandegger @ 2008-07-31 20:17 UTC (permalink / raw)
To: Timur Tabi; +Cc: Scott Wood, Linux I2C, Linuxppc-dev
In-Reply-To: <48921954.4020103@freescale.com>
Timur Tabi wrote:
> Wolfgang Grandegger wrote:
>
>> But clock-frequency, aka bus-frequency, is already used by
>> fsl_get_sys_freq():
>>
>> http://lxr.linux.no/linux+v2.6.26/arch/powerpc/sysdev/fsl_soc.c#L80
>
> So? clock-frequency is a per-node property. I use it in the codec node on the
> 8610 (mpc8610_hpcd.dts). It does not mean "platform clock frequency".
>
>> U-Boot could then fixup that value like bus-frequency() and the i2c-mpc
>> driver simply calls fsl_get_i2c_freq().
>
> This is just more complicated than it needs to be. Why should the I2C driver
> fetch the platform clock and the divider from the parent node, and then do
> additional math, when it could just get the value it needs right from the node
> it's probing?
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?
> Besides, U-Boot does not currently store the divider value. Look at the code
> I've posted twice already - it stores the frequency in i2c1_clk. So now I would
> need to create another variable in the gd_t to store the divider? No thanks.
OK, that's an argument but it's biased by U-Boot.
Wolfgang.
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Timur Tabi @ 2008-07-31 20:19 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: Scott Wood, Linux I2C, Linuxppc-dev
In-Reply-To: <48921DED.6010403@grandegger.com>
Wolfgang Grandegger wrote:
> I'm a bit confused. The frequency of the I2C source clock and the real
> I2C clock frequency are two different things.
There are two frequencies:
1) The frequency of the input clock to the I2C device, after it has gone through
a divider. This is what I call the "I2C clock frequency" and what I think
belongs in the clock-frequency property. This is usually the platform clock
divided by 1, 2, or 3.
2) The speed of the I2C bus, as seen by devices on that bus. This is usually
400KHz.
> The first one is common
> for all I2C devices, the second can be different. What properties would
> you like to use for defining both?
The platform clock has no value to the I2C hardware, so I don't care anything
about it.
>> Besides, U-Boot does not currently store the divider value. Look at the code
>> I've posted twice already - it stores the frequency in i2c1_clk. So now I would
>> need to create another variable in the gd_t to store the divider? No thanks.
>
> OK, that's an argument but it's biased by U-Boot.
As long as a method that is favorable to U-Boot does not put any undo hardship
on non-U-Boot methods, I would say that it is the preferred method.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH] powerpc: i2c-mpc: make speed registers configurable via FDT
From: Timur Tabi @ 2008-07-31 20:19 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: Scott Wood, Linux I2C, Linuxppc-dev
In-Reply-To: <48921EA2.1080600@grandegger.com>
Wolfgang Grandegger wrote:
> Is it not exactly that? For me it's not a per I2C device property, at least.
Of course it's a per-I2C device property. The input frequency to the I2C device
is only seen by the I2C device, and no other device.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Board level compatibility matching
From: Grant Likely @ 2008-07-31 20:19 UTC (permalink / raw)
To: Josh Boyer, linuxppc-dev, devicetree-discuss, Kumar Gala,
Benjamin Herrenschmidt, Segher Boessenkool
This topic keeps coming up, so it is probably time to address it once
and for all.
When it comes to machine level support in arch/powerpc, there seems to
me that there are two levels or machine support.
Level 1 is the board specific stuff. Board X has a, b, and c things
that need to be done for Linux to work correctly, but the fixups are
entirely board specific and will only ever be used on a single board
port. The lite5200 support in arch/powerpc/platforms/52xx is an
example of this kind of board support. It binds on a value in the top
level compatible property.
Level 2 is kind of the generic catch-all machine support for systems
that are unremarkable and don't require any special code to be run.
In most cases, new boards can be supported by this generic code
without any changes to the Linux kernel.
arch/powerpc/platforms/52xx/mpc5200_simple.c is an example here.
mpc5200_simple maintains a list of boards that are known to work with
it.
At the moment, every new board port forces a linux kernel source tree
change, even if it is just adding a single string to the match table.
I'm willing to wager that 99 times out of 100, boards based on the
mpc5200 SoC will want to use the common board support code and that
maintaining an explicit list of supported boards is completely
unnecessary. I expect that the exact same is true for 8xxx and 4xx
SoCs. So, rather than continuing to need to maintain explicit lists,
I propose the following:
- Add a property to the device tree that explicitly specifies the SoC
that the board is based on. Something like 'soc-model =
"fsl,mpc5200b"' would be appropriate. This in and of itself does not
change the usage conventions, it just provides more information about
the hardware. (Another idea is to add a string to the top level
compatible property, but there are still arguments about what
compatible really means in the root node.)
- Prioritize board ports in the arch/powerpc/platforms directory to
identify level-1 machines support from the level-2 ones. Make sure
that level-1 stuff always gets probed before level-2 stuff within each
SoC family. In all likelyhood, this would probably just involve
making sure that board specific machines get linked in before the
catchall machine.
- Change level-2 machine support to bind on soc-model instead of an
explicit compatible list.
Doing so should simplify adding new board ports. In many cases it
would just involve dropping in a new .dts file. However, it retains
the flexability of overriding generic code with platform specific
fixups as the need arises. I know we've been cautious about adding
catch-all bindings to the device tree, and it is a big deal to avoid
adding wildcards to compatible values. However, this solution should
be workable because it doesn't involve stating something that is not
true in the device tree and it maintains the ability to override
cleanly when new bugs are discovered. It also doesn't try to define
wildcard values in compatible or other device tree properties.
Thoughts?
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox