All of lore.kernel.org
 help / color / mirror / Atom feed
From: swarren@wwwdotorg.org (Stephen Warren)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 2/2] USB: doc: Binding document for ehci-platform driver
Date: Mon, 22 Oct 2012 10:07:29 -0600	[thread overview]
Message-ID: <50856F41.7000205@wwwdotorg.org> (raw)
In-Reply-To: <1350771032-11527-3-git-send-email-linux@prisktech.co.nz>

On 10/20/2012 04:10 PM, Tony Prisk wrote:
> Add a binding document for ehci-platform driver.
> 
> Signed-off-by: Tony Prisk <linux@prisktech.co.nz>
> ---
>  .../devicetree/bindings/usb/ehci-platform.txt      |   27 ++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/ehci-platform.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/ehci-platform.txt b/Documentation/devicetree/bindings/usb/ehci-platform.txt
> new file mode 100644
> index 0000000..930b19e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/ehci-platform.txt
> @@ -0,0 +1,27 @@
> +Generic Platform EHCI Controller
> +-----------------------------------------------------
> +
> +Required properties:
> +- compatible : "linux,ehci-platform"

That compatible value doesn't look right. The HW isn't defined by Linux.
The binding is supposed to represent HW, not any single OS's use of that
HW or the way its driver works.

Something like "usb,ehci" might be more appropriate. Certainly, the
value should not be "linux,", nor derived from Linux's driver name.

> +Optional properties:
> +- caps-offset : offset to the capabilities register (default = 0)
> +- has-tt : controller has transaction translator(s).
> +- has-synopsys-hc-bug : controller has the synopsys hc bug

That would normally be determined by the driver based on the particular
compatible value that is in device tree.

> +- no-io-watchdog : controller does not need io watchdog
> +
> +- big-endian : descriptors and registers are both big endian. This
> +  is the equivalent of specifying big-endian-desc and big-endian-regs.
> +OR
> +- big-endian-desc : descriptors are in big-endian format
> +- big-endian-regs : mmio is in big-endian format

Hmmm. That looks odd. Presumably if those properties aren't specified,
the default is little-endian? Shouldn't this be a tri-state: big,
little, native, with default native? I don't know what the EHCI
specification mandates here (and if it does mandate something, the
default should match the specification). Isn't this something that
readl/writel would take care of, or are there cases where the register
endianness of just this one HW block mismatches all other HW blocks?

> +Example:
> +	ehci at d8007c00 {
> +		compatible = "ehci-platform";
> +		reg = <0xd8007c00 0x200>;
> +		interrupts = <43>;
> +		has-tt;
> +	};
> 

WARNING: multiple messages have this Message-ID (diff)
From: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
To: Tony Prisk <linux-ci5G2KO2hbZ+pU9mqzGVBQ@public.gmane.org>
Cc: Greg KH
	<gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
	linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Alan Stern
	<stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>,
	Florian Fainelli
	<florian-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>,
	Grant Likely
	<grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
Subject: Re: [PATCH v2 2/2] USB: doc: Binding document for ehci-platform driver
Date: Mon, 22 Oct 2012 10:07:29 -0600	[thread overview]
Message-ID: <50856F41.7000205@wwwdotorg.org> (raw)
In-Reply-To: <1350771032-11527-3-git-send-email-linux-ci5G2KO2hbZ+pU9mqzGVBQ@public.gmane.org>

On 10/20/2012 04:10 PM, Tony Prisk wrote:
> Add a binding document for ehci-platform driver.
> 
> Signed-off-by: Tony Prisk <linux-ci5G2KO2hbZ+pU9mqzGVBQ@public.gmane.org>
> ---
>  .../devicetree/bindings/usb/ehci-platform.txt      |   27 ++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/ehci-platform.txt
> 
> diff --git a/Documentation/devicetree/bindings/usb/ehci-platform.txt b/Documentation/devicetree/bindings/usb/ehci-platform.txt
> new file mode 100644
> index 0000000..930b19e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/ehci-platform.txt
> @@ -0,0 +1,27 @@
> +Generic Platform EHCI Controller
> +-----------------------------------------------------
> +
> +Required properties:
> +- compatible : "linux,ehci-platform"

That compatible value doesn't look right. The HW isn't defined by Linux.
The binding is supposed to represent HW, not any single OS's use of that
HW or the way its driver works.

Something like "usb,ehci" might be more appropriate. Certainly, the
value should not be "linux,", nor derived from Linux's driver name.

> +Optional properties:
> +- caps-offset : offset to the capabilities register (default = 0)
> +- has-tt : controller has transaction translator(s).
> +- has-synopsys-hc-bug : controller has the synopsys hc bug

That would normally be determined by the driver based on the particular
compatible value that is in device tree.

> +- no-io-watchdog : controller does not need io watchdog
> +
> +- big-endian : descriptors and registers are both big endian. This
> +  is the equivalent of specifying big-endian-desc and big-endian-regs.
> +OR
> +- big-endian-desc : descriptors are in big-endian format
> +- big-endian-regs : mmio is in big-endian format

Hmmm. That looks odd. Presumably if those properties aren't specified,
the default is little-endian? Shouldn't this be a tri-state: big,
little, native, with default native? I don't know what the EHCI
specification mandates here (and if it does mandate something, the
default should match the specification). Isn't this something that
readl/writel would take care of, or are there cases where the register
endianness of just this one HW block mismatches all other HW blocks?

> +Example:
> +	ehci@d8007c00 {
> +		compatible = "ehci-platform";
> +		reg = <0xd8007c00 0x200>;
> +		interrupts = <43>;
> +		has-tt;
> +	};
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  parent reply	other threads:[~2012-10-22 16:07 UTC|newest]

Thread overview: 85+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-20 22:10 [PATCH v2 0/2] Update ehci-platform driver to support devicetree Tony Prisk
2012-10-20 22:10 ` Tony Prisk
2012-10-20 22:10 ` [PATCH v2 1/2] USB: Update EHCI-platform driver to devicetree Tony Prisk
2012-10-20 22:10   ` Tony Prisk
2012-10-21  2:02   ` Alan Stern
2012-10-21  2:02     ` Alan Stern
     [not found]   ` <1350771032-11527-2-git-send-email-linux-ci5G2KO2hbZ+pU9mqzGVBQ@public.gmane.org>
2012-10-22 14:51     ` Alan Stern
2012-10-20 22:10 ` [PATCH v2 2/2] USB: doc: Binding document for ehci-platform driver Tony Prisk
2012-10-20 22:10   ` Tony Prisk
2012-10-21 17:34   ` Florian Fainelli
2012-10-21 17:34     ` Florian Fainelli
2012-10-22 16:07   ` Stephen Warren [this message]
2012-10-22 16:07     ` Stephen Warren
2012-10-22 17:34     ` Alan Stern
2012-10-22 17:34       ` Alan Stern
2012-10-22 17:48       ` Stephen Warren
2012-10-22 17:48         ` Stephen Warren
2012-10-22 19:00         ` Alan Stern
2012-10-22 19:00           ` Alan Stern
2012-10-22 22:10           ` Stephen Warren
2012-10-22 22:10             ` Stephen Warren
2012-10-23 14:10             ` Alan Stern
2012-10-23 14:10               ` Alan Stern
2012-10-23 16:15               ` Stephen Warren
2012-10-23 16:15                 ` Stephen Warren
2012-10-23 17:59                 ` Alan Stern
2012-10-23 17:59                   ` Alan Stern
2012-10-23 18:47                   ` Stephen Warren
2012-10-23 18:47                     ` Stephen Warren
2012-10-23 19:33                     ` Alan Stern
2012-10-23 19:33                       ` Alan Stern
2012-10-23 20:06                       ` Rob Herring
2012-10-23 20:06                         ` Rob Herring
2012-10-24 14:57                         ` Alan Stern
2012-10-24 14:57                           ` Alan Stern
2012-10-24 15:26                           ` Sebastian Andrzej Siewior
2012-10-24 15:26                             ` Sebastian Andrzej Siewior
2012-10-24 16:16                             ` Stephen Warren
2012-10-24 16:16                               ` Stephen Warren
2012-10-24 16:36                               ` Florian Fainelli
2012-10-24 16:36                                 ` Florian Fainelli
2012-10-24 16:38                               ` Alan Stern
2012-10-24 16:38                                 ` Alan Stern
2012-10-24 16:44                                 ` Florian Fainelli
2012-10-24 16:44                                   ` Florian Fainelli
2012-10-24 18:04                                   ` Alan Stern
2012-10-24 18:04                                     ` Alan Stern
2012-10-24 18:18                                     ` Florian Fainelli
2012-10-24 18:18                                       ` Florian Fainelli
2012-10-24 16:45                                 ` Stephen Warren
2012-10-24 16:45                                   ` Stephen Warren
2012-10-24 17:46                                   ` Alan Stern
2012-10-24 17:46                                     ` Alan Stern
2012-10-24 18:09                                     ` Stephen Warren
2012-10-24 18:09                                       ` Stephen Warren
2012-10-24 18:55                                       ` Mitch Bradley
2012-10-24 18:55                                         ` Mitch Bradley
2012-10-24 19:30                                         ` Alan Stern
2012-10-24 19:30                                           ` Alan Stern
2012-10-25 10:23                                         ` Sebastian Andrzej Siewior
2012-10-25 10:23                                           ` Sebastian Andrzej Siewior
2012-10-25 14:36                                           ` Alan Stern
2012-10-25 14:36                                             ` Alan Stern
2012-10-26  8:02                                             ` Sebastian Andrzej Siewior
2012-10-26  8:02                                               ` Sebastian Andrzej Siewior
2012-10-26 14:54                                               ` Alan Stern
2012-10-26 14:54                                                 ` Alan Stern
2012-10-25 15:53                                           ` Stephen Warren
2012-10-25 15:53                                             ` Stephen Warren
2012-10-24 19:41                                       ` Alan Stern
2012-10-24 19:41                                         ` Alan Stern
2012-10-24 16:44                               ` Alan Stern
2012-10-24 16:44                                 ` Alan Stern
2012-10-24 16:48                                 ` Stephen Warren
2012-10-24 16:48                                   ` Stephen Warren
2012-10-24 17:42                                 ` Rob Herring
2012-10-24 17:42                                   ` Rob Herring
2012-10-24 17:57                                   ` Alan Stern
2012-10-24 17:57                                     ` Alan Stern
2012-10-24 16:28                           ` Stephen Warren
2012-10-24 16:28                             ` Stephen Warren
2012-10-24 16:54                             ` Alan Stern
2012-10-24 16:54                               ` Alan Stern
2012-10-24 17:37                               ` Florian Fainelli
2012-10-24 17:37                                 ` Florian Fainelli

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=50856F41.7000205@wwwdotorg.org \
    --to=swarren@wwwdotorg.org \
    --cc=linux-arm-kernel@lists.infradead.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.