All of lore.kernel.org
 help / color / mirror / Atom feed
From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2] USB: Support for LPC32xx SoC
Date: Tue, 28 Feb 2012 15:53:41 +0000	[thread overview]
Message-ID: <201202281553.41332.arnd@arndb.de> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1202280951440.1456-100000@iolanthe.rowland.org>

On Tuesday 28 February 2012, Alan Stern wrote:
> It's heading in the intended direction, although the details might not 
> all be quite right -- I didn't check them very closely.
> 
> One big thing about it is wrong: Many or most of the functions you
> exported don't really need to be.  Instead, ohci-hcd.c should define
> ohci_driver (a bus-agnostic hc_driver structure) and export that.  
> Then the bus-glue files can copy the structure for their own use during
> initialization (rather than duplicating the definition all over the
> place) and override individual methods as needed.
> 
> It's a more "object-oriented" approach.  :-)

Yes, that makes sense. I did not try to actually understand how the
driver works internally, just tried the mechanical conversion in a way
that did not require changing any code besides the sb800_prefetch
function that had to be moved.

There are still a few symbols that are used by most or all hw specific
drivers and that will have to remain exported:

ohci_init, ohci_run, ohci_stop, ohci_finish_controller_resume, and
ohci_hcd_init

And then there are a few symbols that are only used by one or two
drivers, possibly correctly or not:

ohci_dump (spear)
ohci_usb_reset (at91, pci)
ohci_shutdown (ps3)
ohci_restart (pci)
ohci_hub_control (da8xx)

These ones do not need to get exported following your suggestion:

ohci_urb_enqueue, ohci_urb_dequeue, ohci_endpoint_disable,
ohci_get_frame, ohci_irq, ohci_bus_suspend, ohci_bus_resume,
ohci_hub_status_data, and ohci_start_port_reset

I would do implementation the other way round and let the bus specific
driver provide a sparsely populated version of struct hc_driver
that is completed by a function in the common ohci parts. That would
keep the logicto combine the two in one place rather than duplicating
it everywhere, but it's a bit more overhead in the case where you
build only a single bus glue.

Certainly either way is possible, whichever you prefer.

	Arnd

WARNING: multiple messages have this Message-ID (diff)
From: Arnd Bergmann <arnd@arndb.de>
To: linux-arm-kernel@lists.infradead.org
Cc: Alan Stern <stern@rowland.harvard.edu>,
	Roland Stigge <stigge@antcom.de>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	linux-usb@vger.kernel.org, Wolfram Sang <w.sang@pengutronix.de>,
	kevin.wells@nxp.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] USB: Support for LPC32xx SoC
Date: Tue, 28 Feb 2012 15:53:41 +0000	[thread overview]
Message-ID: <201202281553.41332.arnd@arndb.de> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1202280951440.1456-100000@iolanthe.rowland.org>

On Tuesday 28 February 2012, Alan Stern wrote:
> It's heading in the intended direction, although the details might not 
> all be quite right -- I didn't check them very closely.
> 
> One big thing about it is wrong: Many or most of the functions you
> exported don't really need to be.  Instead, ohci-hcd.c should define
> ohci_driver (a bus-agnostic hc_driver structure) and export that.  
> Then the bus-glue files can copy the structure for their own use during
> initialization (rather than duplicating the definition all over the
> place) and override individual methods as needed.
> 
> It's a more "object-oriented" approach.  :-)

Yes, that makes sense. I did not try to actually understand how the
driver works internally, just tried the mechanical conversion in a way
that did not require changing any code besides the sb800_prefetch
function that had to be moved.

There are still a few symbols that are used by most or all hw specific
drivers and that will have to remain exported:

ohci_init, ohci_run, ohci_stop, ohci_finish_controller_resume, and
ohci_hcd_init

And then there are a few symbols that are only used by one or two
drivers, possibly correctly or not:

ohci_dump (spear)
ohci_usb_reset (at91, pci)
ohci_shutdown (ps3)
ohci_restart (pci)
ohci_hub_control (da8xx)

These ones do not need to get exported following your suggestion:

ohci_urb_enqueue, ohci_urb_dequeue, ohci_endpoint_disable,
ohci_get_frame, ohci_irq, ohci_bus_suspend, ohci_bus_resume,
ohci_hub_status_data, and ohci_start_port_reset

I would do implementation the other way round and let the bus specific
driver provide a sparsely populated version of struct hc_driver
that is completed by a function in the common ohci parts. That would
keep the logicto combine the two in one place rather than duplicating
it everywhere, but it's a bit more overhead in the case where you
build only a single bus glue.

Certainly either way is possible, whichever you prefer.

	Arnd

  reply	other threads:[~2012-02-28 15:53 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-23 20:57 [PATCH v2] USB: Support for LPC32xx SoC Roland Stigge
2012-02-23 20:57 ` Roland Stigge
2012-02-23 21:05 ` Roland Stigge
2012-02-23 21:05   ` Roland Stigge
2012-02-24  6:35   ` Wolfram Sang
2012-02-24  6:35     ` Wolfram Sang
2012-02-24 15:03     ` Arnd Bergmann
2012-02-24 15:03       ` Arnd Bergmann
2012-02-25  3:51       ` Wolfram Sang
2012-02-25  3:51         ` Wolfram Sang
2012-02-25  8:45         ` Arnd Bergmann
2012-02-25  8:45           ` Arnd Bergmann
2012-02-25 15:46           ` Alan Stern
2012-02-25 15:46             ` Alan Stern
2012-02-26 10:01             ` Arnd Bergmann
2012-02-26 10:01               ` Arnd Bergmann
2012-02-26 15:59               ` Alan Stern
2012-02-26 15:59                 ` Alan Stern
2012-02-27 14:03                 ` Arnd Bergmann
2012-02-27 14:03                   ` Arnd Bergmann
2012-02-27 16:42                   ` Alan Stern
2012-02-27 16:42                     ` Alan Stern
2012-02-27 22:01                     ` Arnd Bergmann
2012-02-27 22:01                       ` Arnd Bergmann
2012-02-28 15:01                       ` Alan Stern
2012-02-28 15:01                         ` Alan Stern
2012-02-28 15:53                         ` Arnd Bergmann [this message]
2012-02-28 15:53                           ` Arnd Bergmann
2012-02-28 16:51                           ` Alan Stern
2012-02-28 16:51                             ` Alan Stern
2012-02-25 14:03         ` Roland Stigge
2012-02-25 14:03           ` Roland Stigge

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=201202281553.41332.arnd@arndb.de \
    --to=arnd@arndb.de \
    --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.