linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: linux@arm.linux.org.uk (Russell King - ARM Linux)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/5] USB: add clk structure for systems that support clkdev framework
Date: Sun, 2 May 2010 16:14:25 +0100	[thread overview]
Message-ID: <20100502151425.GC4233@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <r2oc70ff3ad1005020805zc80ad8b9n1e56d4a0a6ce5b9c@mail.gmail.com>

On Sun, May 02, 2010 at 06:05:32PM +0300, saeed bishara wrote:
> On Sun, May 2, 2010 at 5:36 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Sun, May 02, 2010 at 05:22:40PM +0300, Saeed Bishara wrote:
> >> @@ -110,6 +111,9 @@ struct usb_hcd {
> >> ? ? ? u64 ? ? ? ? ? ? ? ? ? ? rsrc_start; ? ? /* memory/io resource start */
> >> ? ? ? u64 ? ? ? ? ? ? ? ? ? ? rsrc_len; ? ? ? /* memory/io resource length */
> >> ? ? ? unsigned ? ? ? ? ? ? ? ?power_budget; ? /* in mA, 0 = no limit */
> >> +#if defined(CONFIG_HAVE_CLK)
> >> + ? ? struct clk ? ? ? ? ? ? ?*clk;
> >> +#endif
> >
> > We have other hci's using the clk API, why do we need to add this for
> > them too? ?In other words, why can't the orion HCI driver work like
> > the other HCI drivers such as ohci-pxa27x.c or ehci-omap.c ?
> >
> if most of those drivers need clk structure then why not to add it to
> the usb_hcd which hold the common stuff?

drivers/usb/host/ohci-s3c2410.c:	2
drivers/usb/host/ohci-omap.c:		2
drivers/usb/host/r8a66597-hcd.c:	1
drivers/usb/host/ehci-mxc.c:		2
drivers/usb/host/ohci-da8xx.c:		2
drivers/usb/host/ohci-pxa27x.c:		1
drivers/usb/host/ohci-pnx4008.c:	1
drivers/usb/host/imx21-hcd.c:		1
drivers/usb/host/ohci-ep93xx.c:		1
drivers/usb/host/ehci-atmel.c:		2
drivers/usb/host/ehci-omap.c:		5
drivers/usb/host/ohci-at91.c:		2

So, five drivers need one clock, six drivers need two clocks, and one
driver needs five clocks.  So maybe you should be catering for the
common case by providing two struct clk's, or maybe catering for the
maximal case of five clocks?

> if I get approval for this then we can change the other drivers to use
> this clk instead of each one having its own variable.

That wasn't covered in the original patch - who gets to create these
patches, and how soon will they be created after 'approval' ?

Having these patches with this patch submission may actually help your
case by showing the benefits of such a cleanup - eg, if it eliminates
private driver data structures entirely, it's definitely worth it.

However, if it results in one clk structure in one structure and a
bunch of other clk structures elsewhere, imho it makes very little
sense - I'd go as far as to suggest that it creates confusion.

  reply	other threads:[~2010-05-02 15:14 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-02 14:22 [PATCH 0/5] USB: orion ehci patches for 2.6.35 Saeed Bishara
2010-05-02 14:22 ` [PATCH 1/5] USB: add support for phy init for the Dove SoC Saeed Bishara
2010-05-02 14:22   ` [PATCH 2/5] ARM: use the Dove USB phy setup Saeed Bishara
2010-05-02 14:22     ` [PATCH 3/5] USB: add clk structure for systems that support clkdev framework Saeed Bishara
2010-05-02 14:22       ` [PATCH 4/5] USB: manage the orion ehci clock using the clkdev Saeed Bishara
2010-05-02 14:22         ` [PATCH 5/5] USB: add power management support for orion ehci Saeed Bishara
2010-05-02 14:36       ` [PATCH 3/5] USB: add clk structure for systems that support clkdev framework Russell King - ARM Linux
2010-05-02 15:05         ` saeed bishara
2010-05-02 15:14           ` Russell King - ARM Linux [this message]
2010-05-02 15:21             ` saeed bishara
2010-05-02 15:31               ` Russell King - ARM Linux
2010-05-02 15:54                 ` saeed bishara
2010-05-02 16:05                   ` Russell King - ARM Linux
2010-05-25  7:08                     ` saeed bishara
2010-06-06 10:28                     ` saeed bishara

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=20100502151425.GC4233@n2100.arm.linux.org.uk \
    --to=linux@arm.linux.org.uk \
    --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 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).