linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Gerhard Pircher <gerhard_pircher@gmx.net>
Cc: linuxppc-dev@ozlabs.org
Subject: Re: [RFC] AmigaOne device tree source v2
Date: Mon, 3 Sep 2007 11:34:31 +1000	[thread overview]
Message-ID: <20070903013431.GG31499@localhost.localdomain> (raw)
In-Reply-To: <20070831175006.17240@gmx.net>

On Fri, Aug 31, 2007 at 07:50:06PM +0200, Gerhard Pircher wrote:
> Hi,
> 
> I updated the AmigaOne device tree based on the comments in this thread:
> http://ozlabs.org/pipermail/linuxppc-dev/2007-June/038069.html
> All the ISA devices are now subnodes of the PCI2ISA bridge, which marks
> the first 64k (of PCI address space) as I/O space. The pci node doesn't
> contain any interrupt routing information, because interrupt routing
> differs between the three AmigaOne models. Thus I would like to omit it,
> if it is not really necessary. 

Interrupt routing information is really necessary.  If it differs
between the models then either you will need different device tree
source for each of them, or you will need to fill in the correct
interrupt routing information from the bootwrapper, whichever approach
is easier.

> The PCI host for bus 0 is a subnode of
> the pci node, but I'm not sure if this is correct.
> 
> Please take a look at the reg and ranges properties of the PCI devices.
> The PCI OF spec defined "zero" reg properties (like
> reg = <00xxxx00 00000000 00000000 00000000 00000000>, where xxxx is
> the device number), even if all the other BARs are defined. What are
> they good for?
> The BARs of the VIA IDE controller are assumed to be relocateable,
> even if the address is fixed in compatibility mode.
> BTW: Is there a way to specify the addresses for PCI config with
> indirect addressing?

I believe that's handled by the bridge's compatible and reg
properties.  The platform or bridge code will have to know that this
type of bridge (as encoded in compatible) uses indirect addressing,
and which resource (from the reg property) has the indirect access
registers.

I don't know enough about PCI to answer most of the above questions,
but I spotted a bunch of problems with the device tree anyway...
> /*
>  * AmigaOne Device Tree Source
>  *
>  * Copyright 2007 Gerhard Pircher (gerhard_pircher@gmx.net)
>  *
>  * This program is free software; you can redistribute  it and/or modify it
>  * under  the terms of  the GNU General  Public License as published by the
>  * Free Software Foundation;  either version 2 of the  License, or (at your
>  * option) any later version.
>  */
> 
> 
> / {
> 	model = "Eyetech,AmigaOne";
> 	compatible = "Eyetech,AmigaOne" "MAI,Teron";
> 	#address-cells = <1>;
> 	#size-cells = <1>;
> 
> 	cpus {
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 
> 		cpu@0 {
> 			device_type = "cpu";
> 			reg = <0>;
> 			d-cache-line-size = <20>;	// 32 bytes
> 			i-cache-line-size = <20>;	// 32 bytes
> 			d-cache-size = <8000>;		// L1, 32K
> 			i-cache-size = <8000>;		// L1, 32K
> 			timebase-frequency = <0>;	// 33.3 MHz, from U-boot
> 			clock-frequency = <0>;		// From U-boot
> 			bus-frequency = <0>;		// From U-boot
> 			32-bit;
> 		};
> 	};
> 
> 	memory {
> 		device_type = "memory";
> 		reg = <0 0>;				// From U-boot
> 	};
> 
>   	pci@80000000 {
> 		device_type = "pci";
> 		bus-frequency = <01fca055>;		// 33.3MHz
> 		bus-range = <0 1>;
> 		reg = <80000000 7f000000>;				// Whole PCI space.

'reg' and 'ranges' should not typically overlap.  'reg' should only
encode control registers for the bridge, not the whole PCI space (not
that I'm even entirely sure what you mean by that).

> 		ranges = <01000000 0 00000000 fe000000 0 00c00000	// PCI I/O
> 			  02000000 0 80000000 80000000 0 7d000000	// PCI memory
> 			  02000000 0 fd000000 fd000000 0 01000000>;	// PCI alias memory
> 		8259-interrupt-acknowledge = <fef00000>;
> 		#address-cells = <3>;
> 		#size-cells = <2>;
> 
> 		host@0 {

The unit address (after the @) should be derived from the first range
listed in the 'reg' property.  It's a bus address, not a slot number.

> 			vendor-id = 0x000010cc;

Um.. evidentally you have never even tried compiling this device tree,
since this is invalid syntax.  That would need to be:
			vendor-id = <000010cc>;
and likewise all the others below.

> 			device-id = 0x00000660;
> 			revision-id = 0x00000001;
> 			class-code = 0x00060000;
> 			subsystem-id = 0x00000000;
> 			subsystem-vendor-id = 0x00000000;
> 			devsel-speed = 0x00000001;
> 			66mhz-capable;
> 			min-grant = 0x00000000;
> 			max-latency = 0x00000000;
> 			// AGP aperture is unset.
> 			reg = <42000010 0 00000000 0 00400000>;
> 			assigned-addresses = <42000010 0 00000000 0 00400000>;
> 		}
> 
> 		isa@7 {
> 			device_type = "isa";
> 			vendor-id = 0x00001106;
> 			device-id = 0x00000686;
> 			revision-id = 0x00000010;
> 			class-code = 0x00060100;
> 			subsystem-id = 0x00000000;
> 			subsystem-vendor-id = 0x00000000;
> 			devsel-speed = 0x00000001;
> 			min-grant = 0x00000000;
> 			max-latency = 0x00000000;
> 			/* First 64k for I/O at 0x0 on PCI mapped to 0x0 on ISA. */
> 			ranges = <00000001 0 01000000 0 00000000 00010000>;
> 			interrupt-parent = <&/pci@80000000/isa@7/interrupt-controller>;

Probably worth using a label on the interrupt node to avoid giving a
full path here.

> 			#interrupt-cells = <2>;
> 			#address-cells = <2>;
> 			#size-cells = <1>;
> 
> 			dma-controller {

All these devices should have unit addresses.

> 				device_type = "dma-controller";
> 				compatible = "pnpPNP,200";
> 				reg = <00000001 00000000 00000010
> 				       00000001 00000080 00000010
> 				       00000001 000000c0 00000020>;
> 				/* Channel 4 reserverd, cascade mode, 2x32k transfer/counter
> 				 * widths and bus master capability. Is this really necessary?
> 				 */
> /*				dma = <4 4 20 20 1>; */
> 			};
> 
> 		  	interrupt-controller {
> 				device_type = "interrupt-controller";
> 				compatible = "pnpPNP,000";
> 				interrupt-controller;
> 				reg = <00000001 00000020 00000002
> 				       00000001 000000a0 00000002
> 				       00000001 000004d0 00000002>;
> 				reserved-interrupts = <2>;	
> 			};
> 
> 			8042@60 {
> 				device_type = "8042";
> 				compatible = "pnpPNP,303";
> 				reg = <00000001 00000060 00000010>;
> 				interrupts = <1 3 c 3>;			// IRQ1, IRQ12 (rising edge)
> 
> 				keyboard {
> 					device_type = "keyboard";
> 					compatible = "pnpPNP,303";	// Here again?
> 					reg = <0 0 0>;
> 				};
> 
> 				mouse {
> 					device_type = "mouse";
> 					compatible = "pnpPNP,f03";
> 					reg = <0 0 0>;
> 				};
> 			};
> 
> 			timer@40 {
> /*				device_type = "timer"; */		// No device type binding for now.
> 				compatibe = "pnpPNP,100";		// Also add pcspkr to platform devices.
> 				reg = <00000001 00000040 00000020>;
> 			};
> 
> 			rtc@70 {
> 				device_type = "rtc";
> 				compatible = "pnpPNP,b00";		// <ds1385-rtc>; // What should be used here?
> 				reg = <00000001 00000070 00000002>;
> 				interrupts = <8 3>;
> 			};
> 
> 			serial@2f8 {
> 				device_type = "serial";
> 				compatible = "pnpPNP,501" "pnpPNP,500";	// "ns16550"; add property check to OF serial code.
> 				reg = <00000001 000002f8 00000008>;
> 				interrupts = <3 3>;			// IRQ3 (rising edge)
> 				clock-frequency = <0>;			// Not necessary?
> 			};
> 
> 			serial@3f8 {
> 				device_type = "serial";
> 				compatible = "pnpPNP,501" "pnpPNP,500";	// "ns16550"; add property check to OF serial code.
> 				reg = <00000001 000003f8 00000008>;
> 				interrupts = <4 3>;			// IRQ4 (rising edge)
> 				clock-frequency = <0>;			// Not necessary?
> 			};
> 
> 			parallel@378 {
> 				device_type = "parallel";
> 				compatible = "pnpPNP,400"; 		// "pnpPNP,401"	// No ECP support for now.
> 				reg = <00000001 00000378 00000003
> 				       00000001 00000778 00000003>;
> /*				interrupts = <7>; */			// No IRQ free on AmigaOne!
> /*				dma = <3 0 0 0>; */			// Parallel port DMA mode?
> 			};
> 
> 			fdc@3f0 {
> 				device_type = "fdc";
> 				compatible = "pnpPNP,700";
> 				reg = <00000001 000003f0 00000008>;
> 				interrupts = <6 3>;			// IRQ6 (rising edge)
> /*				dma = < >; */				// Floppy DMA mode?
> 
> 				disk@0 {
> 					device_type = "block";
> 					reg = <0 0 0>;
> 				};
> 			};
> 		};
> 
> 		ide@7,1 {

This will need a compatible property, at least.

> 			// Is there a device_type defined for IDE controllers?
> 			vendor-id = 0x00001106;
> 			device-id  = 0x00000571;
> 			revision-id = 0x00000006;
> 			// Class code with PCI IDE programming interface indicator.
> 			class-code = 0x0001018f;
> 			subsystem-id = 0;
> 			subsystem-vendor-id = 0;
> 			devsel-speed = 0x00000001;
> 			min-grant = 0;
> 			max-latency = 0;
> 			fast-back-to-back;
> 			// Assume base addresses are relocateable, even if
> 			// controller operates in compatibility mode. Right?
> 			reg = <21003910 0 00000000 0 00000000
> 			       21003914 0 00000000 0 00000000
> 			       21003918 0 00000000 0 00000000
> 			       2100391c 0 00000000 0 00000000
> 			       21003920 0 00000000 0 00000000>;
> 			assigned-addresses = <01003910 0 000001f0 0 00000008
> 					      01003914 0 000003f4 0 00000004
> 					      01003918 0 00000170 0 00000008
> 					      0100391c 0 00000374 0 00000004
> 					      01003920 0 0000cc00 0 00000010>;
> 	};
> 
> 	chosen {
> 		linux,stdout-path = "/pci@80000000/isa@7/serial@2f8";
> 	};
> };
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

  reply	other threads:[~2007-09-03  1:34 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-08-31 17:50 [RFC] AmigaOne device tree source v2 Gerhard Pircher
2007-09-03  1:34 ` David Gibson [this message]
2007-09-03  8:41   ` Benjamin Herrenschmidt
2007-09-03 10:02     ` Segher Boessenkool
2007-09-03 10:12       ` David Gibson
2007-09-03 16:11         ` Gerhard Pircher
2007-09-03 22:52           ` Segher Boessenkool
2007-09-04  0:27             ` David Gibson
2007-09-06 13:31               ` Segher Boessenkool
2007-09-04 12:20             ` Gerhard Pircher
2007-09-06 13:41               ` Segher Boessenkool
2007-09-03 14:58   ` Gerhard Pircher
2007-09-03 22:32     ` Segher Boessenkool
2007-09-04 11:49       ` Gerhard Pircher
2007-09-05  2:48         ` David Gibson
2007-09-05 11:54           ` Gerhard Pircher
2007-09-06 14:00             ` Segher Boessenkool
2007-09-06 14:09               ` Sven Luther
2007-09-06 14:42                 ` Segher Boessenkool
2007-09-06 13:56           ` Segher Boessenkool
2007-09-06 14:15             ` PCI I/O space -- reg or ranges? Scott Wood
2007-09-06 20:51               ` Gerhard Pircher
2007-09-06 21:01                 ` Segher Boessenkool
2007-09-07  0:20             ` [RFC] AmigaOne device tree source v2 David Gibson
2007-09-06 13:36         ` Segher Boessenkool
2007-09-06 21:09           ` Gerhard Pircher
2007-09-07  0:21           ` David Gibson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20070903013431.GG31499@localhost.localdomain \
    --to=david@gibson.dropbear.id.au \
    --cc=gerhard_pircher@gmx.net \
    --cc=linuxppc-dev@ozlabs.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).