LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: Grant Likely <grant.likely@secretlab.ca>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	sodaville@linutronix.de, devicetree-discuss@lists.ozlabs.org,
	x86@kernel.org, linux-kernel@vger.kernel.org,
	David Gibson <david@gibson.dropbear.id.au>
Subject: Re: [sodaville] [PATCH TIP v2 03/14] x86/dtb: Add a device tree for CE4100
Date: Tue, 22 Feb 2011 12:21:12 +0100	[thread overview]
Message-ID: <20110222112112.GA24641@www.tglx.de> (raw)
In-Reply-To: <20110216214428.GD22837@angua.secretlab.ca>

* Grant Likely | 2011-02-16 14:44:28 [-0700]:

>> diff --git a/arch/x86/platform/ce4100/falconfalls.dts b/arch/x86/platform/ce4100/falconfalls.dts
>> new file mode 100644
>> index 0000000..e888657
>> --- /dev/null
>> +++ b/arch/x86/platform/ce4100/falconfalls.dts
>> @@ -0,0 +1,424 @@
>> +	soc@0 {
>> +		#address-cells = <1>;
>> +		#size-cells = <1>;
>> +		compatible = "intel,ce4100-cp";
>
>You *must* include documentation for each of these new compatible
>values before merging this patch.  At the very least it acts as a
>placeholder and describes exactly what the string describes.

I added something.

>> +		ranges;
>> +
>> +		ioapic1: interrupt-controller@fec00000 {
>> +			#interrupt-cells = <2>;
>> +			compatible = "intel,ioapic-ce4100", "intel,ioapic";
>
>"intel,ioapic" is probably too generic and can be dropped.  Newer
>devices can claim compatibility with "intel,ioapic-ce4100" if they are
>indeed compatible so that device drivers don't need to be modified.
>It is better to anchor compatible values to real implementations that
>try to come up with 'generic' or wildcard strings.  Ditto through the
>rest of the file.
>
>"intel,ce4100-ioapic" (instead of intel,ioapic-ce4100) would follow
>current convention better of specifying the chip first, followed by
>the on-chip device.
>
Done.

>> +			pci@1,0 {
>> +				#address-cells = <3>;
>> +				#interrupt-cells = <1>;
>> +				#size-cells = <2>;
>> +				compatible = "intel,ce4100-pci", "pci";
>> +				device_type = "pci";
>> +				bus-range = <1 1>;
>> +				ranges = <0x2000000 0 0xdffe0000 0x2000000 0 0xdffe0000 0 0x1000>;
>> +
>> +				interrupt-parent = <&ioapic2>;
>> +
>> +				display@2,0 {
>> +					compatible = "pci8086,2e5b.2",
>> +						   "pci8086,2e5b",
>> +						   "pciclass038000",
>> +						   "pciclass0380";
>> +
>> +					reg = <0x11000 0x0 0x0 0x0 0x0>;
>> +					interrupts = <0 1>;
>> +				};
>
>Here's where things get a little sticky for PCI child devices
>(probably should have thought about this earlier, sorry).  Are these
>devices runtime detectable?  ie. does reading the PCI BARs and irq
>configuration registers reflect reality?

Bar yes (no assigned-property here), IRQ no.

This devices are runtime detectable via the PCI bus. However the
interrupt number is no longer correct. The PCI bus reports the legacy
number which is irq 4 (if I remember correctly) for all devices here.
Which is correct for the legacy mode. Those interrupts are routed to the
legacy interrupt controller. Once we activate the IO APIC for interrupt
routing the legacy controller is no longer in use. With the IO APIC
there is a different interrupt routing which is described here by the
interrupt property.
On systems with ACPI this kind of information is read fron there. You
see during boot-up "PCI->APIC IRQ transform: ...".

>In general, if something can be *reliably* detected then it should not
>be listed in the device tree.  However, if there is some horrid gotcha
>that prevents it being detected reliably, then it definitely should
>appear here.

The first edition used the interrupt-map property to describe this
mapping. David Gibson [0] recommended then to create device nodes
because the PCI INTA..D lines were not used.

>I cannot answer this question for you since I don't know the hardware.
>It's a judgement call that you'll need to make on whether or not these
>PCI (or pseudo pci?) devices should be listed in the dt.

Most of the devices can be detected reliably. I2C and SPI need some
additional information. I believe more of this things will pop up once
someone adds the GPIO wires which are not there yet. I know that the
smartcard thingy uses one GPIO for card detect. So I would need to
extend the device property later for that :)

>One of the things that does need to be improved is partial probing of
>PCI busses so that only the messy PCI devices get listed in the device
>tree, and the rest of the devices can get detected in the normal
>manner.

I did not add any "special" compatible properties just the pci ids. So
they should not be probed via DT just PCI unless one adds this to the
driver.

[0] http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-January/004137.html

Sebastian

  reply	other threads:[~2011-02-22 11:21 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-24  4:28 Device tree on x86, part v3 Sebastian Andrzej Siewior
2011-01-24  4:28 ` [PATCH TIP 01/14] x86/e820: remove conditional early mapping in parse_e820_ext Sebastian Andrzej Siewior
2011-02-03 20:57   ` Grant Likely
2011-01-24  4:28 ` [PATCH TIP 02/14] x86: Add device tree support Sebastian Andrzej Siewior
2011-01-24  4:34   ` Sebastian Andrzej Siewior
2011-02-16 21:27     ` Grant Likely
2011-02-17 11:05       ` [sodaville] " Sebastian Andrzej Siewior
2011-02-16 21:26   ` Grant Likely
2011-02-17 11:03     ` Sebastian Andrzej Siewior
2011-02-16 21:31   ` Grant Likely
2011-02-17 11:31     ` [sodaville] " Sebastian Andrzej Siewior
2011-02-17 17:02       ` Grant Likely
2011-01-24  4:28 ` [PATCH TIP 03/14] x86/dtb: Add a device tree for CE4100 Sebastian Andrzej Siewior
2011-01-27  5:00   ` David Gibson
2011-01-27  9:11     ` Sebastian Andrzej Siewior
2011-02-03 20:59       ` Grant Likely
2011-02-03 21:32         ` Mitch Bradley
2011-02-04  9:40           ` Sebastian Andrzej Siewior
2011-02-02 18:58     ` [PATCH TIP v2 " Sebastian Andrzej Siewior
2011-02-03 21:07       ` Grant Likely
2011-02-04 10:06         ` Sebastian Andrzej Siewior
2011-02-16 21:44       ` Grant Likely
2011-02-22 11:21         ` Sebastian Andrzej Siewior [this message]
2011-01-24  4:28 ` [PATCH TIP 04/14] x86/dtb: add irq domain abstraction Sebastian Andrzej Siewior
2011-01-24  4:28 ` [PATCH TIP 05/14] x86/dtb: add early parsing of APIC and IO APIC Sebastian Andrzej Siewior
2011-02-16 21:47   ` Grant Likely
2011-01-24  4:28 ` [PATCH TIP 06/14] x86/dtb: add support hpet Sebastian Andrzej Siewior
2011-01-24  4:28 ` [PATCH OF 07/14] of: move of_irq_map_pci() into generic code Sebastian Andrzej Siewior
2011-02-10 13:57   ` Michal Simek
2011-02-16 21:53   ` Grant Likely
2011-02-17  7:49     ` Michal Simek
2011-01-24  4:28 ` [PATCH TIP 08/14] x86/dtb: add support for PCI devices backed by dtb nodes Sebastian Andrzej Siewior
2011-02-16 21:59   ` Grant Likely
2011-02-22 11:21     ` [sodaville] " Sebastian Andrzej Siewior
2011-01-24  4:28 ` [PATCH TIP 09/14] x86/dtb: Add generic bus probe Sebastian Andrzej Siewior
2011-02-04 10:21   ` [PATCH v2 " Sebastian Andrzej Siewior
2011-02-16 22:04     ` Grant Likely
2011-01-24  4:28 ` [PATCH TIP 10/14] x86/ioapic: Add OF bindings for IO-APIC Sebastian Andrzej Siewior
2011-02-16 22:04   ` Grant Likely
2011-01-24  4:28 ` [PATCH TIP 11/14] x86/ce4100: use OF for ioapic Sebastian Andrzej Siewior
2011-01-24  4:29 ` [PATCH OF 12/14] x86/rtc: don't register rtc if we the DT blob Sebastian Andrzej Siewior
2011-02-16 22:08   ` Grant Likely
2011-02-16 22:09   ` Grant Likely
2011-02-17 13:13     ` [sodaville] " Sebastian Andrzej Siewior
2011-01-24  4:29 ` [PATCH OF 13/14] rtc/cmos: add OF bindings Sebastian Andrzej Siewior
2011-01-24  4:38   ` Sebastian Andrzej Siewior
2011-02-16 22:11   ` Grant Likely
2011-02-17 13:26     ` [sodaville] " Sebastian Andrzej Siewior
2011-02-17 16:46       ` Grant Likely
2011-01-24  4:29 ` [PATCH TIP 14/14] x86/pci: remove warning Sebastian Andrzej Siewior

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=20110222112112.GA24641@www.tglx.de \
    --to=bigeasy@linutronix.de \
    --cc=david@gibson.dropbear.id.au \
    --cc=devicetree-discuss@lists.ozlabs.org \
    --cc=grant.likely@secretlab.ca \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sodaville@linutronix.de \
    --cc=x86@kernel.org \
    --subject='Re: [sodaville] [PATCH TIP v2 03/14] x86/dtb: Add a device tree for CE4100' \
    /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

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).