public inbox for linuxppc-dev@ozlabs.org 
 help / color / mirror / Atom feed
From: Valentine Barshak <vbarshak@ru•mvista.com>
To: Segher Boessenkool <segher@kernel•crashing.org>
Cc: linuxppc-dev@ozlabs•org, linux-usb-devel@lists•sourceforge.net
Subject: Re: [PATCH 2/3] usb: ehci-ppc-of dts bindings.
Date: Wed, 19 Sep 2007 17:52:26 +0400	[thread overview]
Message-ID: <46F1299A.1000508@ru.mvista.com> (raw)
In-Reply-To: <19e5f15308dfb8e60e2e49379666d00a@kernel.crashing.org>

Segher Boessenkool wrote:
>> +  l) USB EHCI controllers
>> +
>> +  Required properties:
>> +  - device_type : should be "usb".
> 
> No device_type please.  The published USB binding doesn't define
> one on purpose.
> 

Could you please, explain why?
Sorry, I don't think I get the concept of device description here.

>> +  - compatible : should be "ehci".
> 
> Just "ehci" isn't enough -- compare to OHCI, which is the name for
> a kind of USB host controller as well as for a kind of Firewire
> host controller.

Actually, I though device type="usb" + compatible="ehci" would be enough.

> 
> Maybe "usb-ehci" is best -- can anyone think of a better name?

Again, why not type="usb", compatible="ehci"?

> 
>> +  - reg : Offset and length of the register set for the device
> 
> Address and length.  You also should declare here how the optional
> EHCI register blocks should be encoded (the USB debug port, etc.)
> 
>> +  - interrupts : <a b> where a is the interrupt number and b is a
>> +    field that represents an encoding of the sense and level
>> +    information for the interrupt.
> 
> This is incorrect; not all interrupt domains use two cells, and
> not all that do have this meaning for those cells.

Well, this was just copied from other descriptions in 
Documentation/powerpc/booting-without-of.txt
Do we need to fix them all?

> 
> Instead, you should just say how many interrupts should be here,
> and which is which in the EHCI standard.
> 
>> +  - interrupt-parent : the phandle for the interrupt controller that
>> +    services interrupts for this device.
> 
> Not every EHCI node needs this; just don't mention this at all,
> interrupt mapping is fully defined for all devices elsewhere
> already (in the "interrupt mapping" recommended practice).
> 
>> +  If device registers are implemented in big endian mode, the device
>> +  node should have "big-endian" property.
>> +  If controller implementation operates with big endian descriptors,
>> +  compatible should also have "ehci-be-desc"
> 
> Ah, I understand what this is about, finally.
> 
> Don't put this in "compatible"; instead, do a "big-endian-descriptors"
> property similar to the "big-endian" property.  That last one should
> maybe be "big-endian-registers" here then, to avoid confusion.
> 
> Or make "big-endian" mean both big-endian registers *and* big-endian
> descriptors.

But doesn't "big-endian" property actually mean "big-endian-registers"?
Do we have to overload property meaning in this case?

> 
> I have no opinion which is best; it depends on what configurations
> actually exist, and how popular those are.
> 
>> +    ehci@e0000300 {
>> +        device_type = "usb";
>> +        compatible = "ibm,ehci-440epx", "ehci-be-desc", "ehci";
>> +        interrupts = <1a 4>;
>> +        interrupt-parent = <&UIC0>;
>> +        reg = <0 e0000300 ff>;
> 
> Length should be 100 here.
> 
>> +        big-endian;
>> +    };
> 
> 
> Segher
> 

Thanks,
Valentine.

  reply	other threads:[~2007-09-19 13:53 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-09-17 12:50 [PATCH 0/3] usb: ehci ppc device-tree-aware driver Valentine Barshak
2007-09-17 12:55 ` [PATCH 1/3] usb: add device-tree-aware ehci driver Valentine Barshak
2007-09-17 13:00   ` [PATCH 2/3] usb: ehci-ppc-of dts bindings Valentine Barshak
2007-09-19  0:25     ` Segher Boessenkool
2007-09-19 13:52       ` Valentine Barshak [this message]
2007-09-19 16:50         ` Segher Boessenkool
2007-09-19 16:55           ` Valentine Barshak
2007-09-24 19:32             ` [linux-usb-devel] " Valentine Barshak
2007-09-20  0:52           ` David Gibson
2007-09-24 21:40             ` Segher Boessenkool
2007-09-17 13:28   ` [PATCH 1/3] usb: add device-tree-aware ehci driver Stephen Rothwell
2007-09-17 14:00     ` Josh Boyer
2007-09-17 18:17     ` Valentine Barshak
2007-09-18  4:26       ` Stephen Rothwell
2007-09-18  5:39         ` David Gibson
2007-09-17 18:18     ` Valentine Barshak
2007-09-17 13:02 ` [PATCH 3/3] Add PowerPC 440EPx Sequoia ehci dts entry Valentine Barshak
2007-09-22 23:00 ` [PATCH 0/3] usb: ehci ppc device-tree-aware driver Hollis Blanchard
2007-09-24 10:33   ` Valentine Barshak
2007-10-08 18:15   ` Jerone Young
2007-10-08 18:18     ` Valentine Barshak
2007-10-08 18:22       ` Valentine Barshak
  -- strict thread matches above, loose matches on Subject: below --
2007-09-24 19:25 Valentine Barshak
2007-09-24 19:27 ` [PATCH 2/3] usb: ehci-ppc-of dts bindings Valentine Barshak

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=46F1299A.1000508@ru.mvista.com \
    --to=vbarshak@ru$(echo .)mvista.com \
    --cc=linux-usb-devel@lists$(echo .)sourceforge.net \
    --cc=linuxppc-dev@ozlabs$(echo .)org \
    --cc=segher@kernel$(echo .)crashing.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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