This patch adds OHCI support to the LPC32xx ARM platform
Signed-off-by: Roland Stigge <[email protected]>
---
drivers/usb/host/ohci-hcd.c | 5 +++
drivers/usb/host/ohci-pnx4008.c | 53 +++++++++++++++++++++++++++++++++++++++-
2 files changed, 57 insertions(+), 1 deletion(-)
--- linux-2.6.orig/drivers/usb/host/ohci-hcd.c
+++ linux-2.6/drivers/usb/host/ohci-hcd.c
@@ -1055,6 +1055,11 @@ MODULE_LICENSE ("GPL");
#define PLATFORM_DRIVER usb_hcd_pnx4008_driver
#endif
+#ifdef CONFIG_ARCH_LPC32XX
+#include "ohci-pnx4008.c"
+#define PLATFORM_DRIVER usb_hcd_pnx4008_driver
+#endif
+
#ifdef CONFIG_ARCH_DAVINCI_DA8XX
#include "ohci-da8xx.c"
#define PLATFORM_DRIVER ohci_hcd_da8xx_driver
--- linux-2.6.orig/drivers/usb/host/ohci-pnx4008.c
+++ linux-2.6/drivers/usb/host/ohci-pnx4008.c
@@ -143,8 +143,21 @@ static void i2c_write(u8 buf, u8 subaddr
i2c_master_send(isp1301_i2c_client, &tmpbuf[0], 2);
}
+#ifdef CONFIG_ARCH_LPC32XX
+static u16 i2c_read16(u8 subaddr)
+{
+ u16 data;
+
+ i2c_master_send(isp1301_i2c_client, &subaddr, 1);
+ i2c_master_recv(isp1301_i2c_client, (u8 *) &data, 2);
+
+ return data;
+}
+#endif
+
static void isp1301_configure(void)
{
+#if !defined(CONFIG_ARCH_LPC32XX)
/* PNX4008 only supports DAT_SE0 USB mode */
/* PNX4008 R2A requires setting the MAX603 to output 3.6V */
/* Power up externel charge-pump */
@@ -166,7 +179,41 @@ static void isp1301_configure(void)
ISP1301_I2C_INTERRUPT_FALLING | ISP1301_I2C_REG_CLEAR_ADDR);
i2c_write(0xFF,
ISP1301_I2C_INTERRUPT_RISING | ISP1301_I2C_REG_CLEAR_ADDR);
-
+#else
+ /* LPC32XX only supports DAT_SE0 USB mode */
+ /* This sequence is important */
+
+ /* Disable transparent UART mode first */
+ i2c_write(MC1_UART_EN, (ISP1301_I2C_MODE_CONTROL_1 |
+ ISP1301_I2C_REG_CLEAR_ADDR));
+
+ i2c_write(~MC1_SPEED_REG, (ISP1301_I2C_MODE_CONTROL_1 |
+ ISP1301_I2C_REG_CLEAR_ADDR));
+ i2c_write(MC1_SPEED_REG, ISP1301_I2C_MODE_CONTROL_1);
+ i2c_write(~0,
+ (ISP1301_I2C_MODE_CONTROL_2 | ISP1301_I2C_REG_CLEAR_ADDR));
+ i2c_write((MC2_BI_DI | MC2_PSW_EN | MC2_SPD_SUSP_CTRL),
+ ISP1301_I2C_MODE_CONTROL_2);
+ i2c_write(~0, (ISP1301_I2C_OTG_CONTROL_1 | ISP1301_I2C_REG_CLEAR_ADDR));
+ i2c_write(MC1_DAT_SE0, ISP1301_I2C_MODE_CONTROL_1);
+ i2c_write((OTG1_DM_PULLDOWN | OTG1_DP_PULLDOWN),
+ ISP1301_I2C_OTG_CONTROL_1);
+ i2c_write((OTG1_DM_PULLUP | OTG1_DP_PULLUP),
+ (ISP1301_I2C_OTG_CONTROL_1 | ISP1301_I2C_REG_CLEAR_ADDR));
+ i2c_write(~0,
+ ISP1301_I2C_INTERRUPT_LATCH | ISP1301_I2C_REG_CLEAR_ADDR);
+ i2c_write(~0,
+ ISP1301_I2C_INTERRUPT_FALLING | ISP1301_I2C_REG_CLEAR_ADDR);
+ i2c_write(~0,
+ ISP1301_I2C_INTERRUPT_RISING | ISP1301_I2C_REG_CLEAR_ADDR);
+
+ /* Enable usb_need_clk clock after transceiver is initialized */
+ __raw_writel((__raw_readl(USB_CTRL) | (1 << 22)), USB_CTRL);
+
+ printk(KERN_INFO "ISP1301 Vendor ID : 0x%04x\n", i2c_read16(0x00));
+ printk(KERN_INFO "ISP1301 Product ID : 0x%04x\n", i2c_read16(0x02));
+ printk(KERN_INFO "ISP1301 Version ID : 0x%04x\n", i2c_read16(0x14));
+#endif
}
static inline void isp1301_vbus_on(void)
@@ -257,6 +304,7 @@ static const struct hc_driver ohci_pnx40
static void pnx4008_set_usb_bits(void)
{
+#if !defined(CONFIG_ARCH_LPC32XX)
start_int_set_falling_edge(SE_USB_OTG_ATX_INT_N);
start_int_ack(SE_USB_OTG_ATX_INT_N);
start_int_umask(SE_USB_OTG_ATX_INT_N);
@@ -280,16 +328,19 @@ static void pnx4008_set_usb_bits(void)
start_int_set_rising_edge(SE_USB_AHB_NEED_CLK_INT);
start_int_ack(SE_USB_AHB_NEED_CLK_INT);
start_int_umask(SE_USB_AHB_NEED_CLK_INT);
+#endif
}
static void pnx4008_unset_usb_bits(void)
{
+#if !defined(CONFIG_ARCH_LPC32XX)
start_int_mask(SE_USB_OTG_ATX_INT_N);
start_int_mask(SE_USB_OTG_TIMER_INT);
start_int_mask(SE_USB_I2C_INT);
start_int_mask(SE_USB_INT);
start_int_mask(SE_USB_NEED_CLK_INT);
start_int_mask(SE_USB_AHB_NEED_CLK_INT);
+#endif
}
static int __devinit usb_hcd_pnx4008_probe(struct platform_device *pdev)
On 23/02/12 21:57, Roland Stigge wrote:
> This patch adds OHCI support to the LPC32xx ARM platform
To get rid of the remaining #ifdefs, I could as well create a new
ohci-lpc32xx.c instead of abusing ohci-pnx4008.c. Would this be the way
to go or are there any other suggestions?
Thanks in advance,
Roland
> To get rid of the remaining #ifdefs, I could as well create a new
> ohci-lpc32xx.c instead of abusing ohci-pnx4008.c. Would this be the way
> to go or are there any other suggestions?
If you could tell the difference between pnx4008 and lpc32xx, then you could
simply do something like:
if (machine_is_pnx4008)
pnx4008_configure();
else
lpc32xx_configure();
or
if (machine_is_pnx4008)
pnx4008_(un)set_usb_bits
Regards,
Wolfram
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
On Friday 24 February 2012, Wolfram Sang wrote:
> > To get rid of the remaining #ifdefs, I could as well create a new
> > ohci-lpc32xx.c instead of abusing ohci-pnx4008.c. Would this be the way
> > to go or are there any other suggestions?
>
> If you could tell the difference between pnx4008 and lpc32xx, then you could
> simply do something like:
>
> if (machine_is_pnx4008)
> pnx4008_configure();
> else
> lpc32xx_configure();
>
> or
>
> if (machine_is_pnx4008)
> pnx4008_(un)set_usb_bits
I would recomment telling this from the device itself rather than from the
platform. You can add a platform_device_id list and put information like this
into the driver_data, then do
switch (pdev->id.driver_data) {
case OHCI_PNX4008:
pnx4008_configure();
break;
case
lpc32xx_configure();
break;
};
if (pdev->id.driver_data == OHCI_PNX4008)
pnx4008_(un)set_usb_bits()
Alternatively, you can put the common code into one file, and use separate
files for the parts that are different between pnx4008 and lpc32xx.
Unfortunately, the way that ohci handles the abstraction between the
various implementations is backwards, it would be much easier if the
main driver was following that model to start with.
Arnd
> I would recomment telling this from the device itself rather than from the
> platform.
Basically yes...
> Unfortunately, the way that ohci handles the abstraction between the
> various implementations is backwards, it would be much easier if the
> main driver was following that model to start with.
... exactly for that reason I didn't suggest it. Looking at other ohci-sources
(e.g. ohci-omap.c), machine_is_* seems to be the consistent use-case.
Thanks,
Wolfram
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
On Saturday 25 February 2012, Wolfram Sang wrote:
> > Unfortunately, the way that ohci handles the abstraction between the
> > various implementations is backwards, it would be much easier if the
> > main driver was following that model to start with.
>
> ... exactly for that reason I didn't suggest it. Looking at other ohci-sources
> (e.g. ohci-omap.c), machine_is_* seems to be the consistent use-case.
I still hope that someone takes the time to convert ohci and ehci into
something more regular, building the .c files separately instead of the
conditional #include "foo.c" inside of the main file.
Alan and Greg, do you know if anyone is working on that, and if not do you
think someone should do it?
Arnd
On 25/02/12 04:51, Wolfram Sang wrote:
>> Unfortunately, the way that ohci handles the abstraction between
>> the various implementations is backwards, it would be much easier
>> if the main driver was following that model to start with.
>
> ... exactly for that reason I didn't suggest it. Looking at other
> ohci-sources (e.g. ohci-omap.c), machine_is_* seems to be the
> consistent use-case.
OK, considering this, I for now used machine_is_*() to get rid of the
#ifdefs.
Using platform_device_id.driver_data as Arnd suggested is also a good
idea, and switching to it will be easy when it is due.
Thanks in advance,
Roland
On Sat, 25 Feb 2012, Arnd Bergmann wrote:
> On Saturday 25 February 2012, Wolfram Sang wrote:
> > > Unfortunately, the way that ohci handles the abstraction between the
> > > various implementations is backwards, it would be much easier if the
> > > main driver was following that model to start with.
> >
> > ... exactly for that reason I didn't suggest it. Looking at other ohci-sources
> > (e.g. ohci-omap.c), machine_is_* seems to be the consistent use-case.
>
> I still hope that someone takes the time to convert ohci and ehci into
> something more regular, building the .c files separately instead of the
> conditional #include "foo.c" inside of the main file.
>
> Alan and Greg, do you know if anyone is working on that, and if not do you
> think someone should do it?
You are talking about two separate issues. One is the way the various
bus-glue files get built into the driver, and the other is the way it
#includes .c files.
I don't view the second as a big deal. A few people have complained
about it, but I don't see the point. Sure, it has the disadvantage
that you have to recompile the entire driver any time one of the files
is changed. I can live with that. It has the advantage that symbols
shared among the source files can be static; they don't have to get
merged into the overall kernel namespace when the driver isn't build as
a module.
The first issue is more serious. There are long-term plans to
restructure both of those drivers so that the bus glue resides in a
separate module from the main core of the driver. However I have not
started to work on that yet; there are other more pressing matters to
do first.
It doesn't seem extremely urgent. Things are working the way they are.
The main advantage to restructuring is that it would allow distributors
to build bus glue for multiple platforms in a single distribution
image. Currently that's not possible (except that any one of the
platform glues can be enabled along with the PCI glue).
A little progress has been made already. We just received a submission
for a "generic" platform bus glue file that will be able to take over
the jobs of several of the existiing files. If anyone wants to take
this further I won't object, but I don't plan to work on it myself
soon.
Alan Stern
On Saturday 25 February 2012, Alan Stern wrote:
> You are talking about two separate issues. One is the way the various
> bus-glue files get built into the driver, and the other is the way it
> #includes .c files.
>
> I don't view the second as a big deal. A few people have complained
> about it, but I don't see the point. Sure, it has the disadvantage
> that you have to recompile the entire driver any time one of the files
> is changed.
> I can live with that. It has the advantage that symbols
> shared among the source files can be static; they don't have to get
> merged into the overall kernel namespace when the driver isn't build as
> a module.
Right, I don't really object that part (including .c files), with
conditional includes I was referring to the inclusion of hw-specific
glues into the driver.
> The first issue is more serious. There are long-term plans to
> restructure both of those drivers so that the bus glue resides in a
> separate module from the main core of the driver. However I have not
> started to work on that yet; there are other more pressing matters to
> do first.
Ok.
> It doesn't seem extremely urgent. Things are working the way they are.
> The main advantage to restructuring is that it would allow distributors
> to build bus glue for multiple platforms in a single distribution
> image. Currently that's not possible (except that any one of the
> platform glues can be enabled along with the PCI glue).
We are doing a major rework of the ARM architecture tree right now
to allow building multiple SoC platforms together, which has not
been possible traditionally. The "PLATFORM_DRIVER" macro in ohci
and ehci is one of many things standing in the way still and will have
to be dealt with at some point.
> A little progress has been made already. We just received a submission
> for a "generic" platform bus glue file that will be able to take over
> the jobs of several of the existiing files. If anyone wants to take
> this further I won't object, but I don't plan to work on it myself
> soon.
Ok, good to know. The approach of a generic glue is exactly what I
was going to suggest. As we get to ARM platforms that are currently
using PLATFORM_DRIVER, I will then ask people to convert the ohci
glue to use that generic glue instead of adding another *_DRIVER
macro to ohci/ehci.
I'm not sure about what we should do for lpc32xx. Is that
generic glue going into v3.4? If so, we could convert the
pnx4008 driver to use that and abstract it in a way that works
nicely for pnx4008 and lpc32xx. OTOH, we might just let this
one go in as before and ask people to do it right for the next
one.
Arnd
On Sun, 26 Feb 2012, Arnd Bergmann wrote:
> We are doing a major rework of the ARM architecture tree right now
> to allow building multiple SoC platforms together, which has not
> been possible traditionally. The "PLATFORM_DRIVER" macro in ohci
> and ehci is one of many things standing in the way still and will have
> to be dealt with at some point.
Is there anybody in particular who is going to attack this issue?
> > A little progress has been made already. We just received a submission
> > for a "generic" platform bus glue file that will be able to take over
> > the jobs of several of the existiing files. If anyone wants to take
> > this further I won't object, but I don't plan to work on it myself
> > soon.
>
> Ok, good to know. The approach of a generic glue is exactly what I
> was going to suggest. As we get to ARM platforms that are currently
> using PLATFORM_DRIVER, I will then ask people to convert the ohci
> glue to use that generic glue instead of adding another *_DRIVER
> macro to ohci/ehci.
The problem is that several platforms have highly specialized needs
that can't sanely be handled in a single generic driver. Although a
lot of drivers can be converted over, not all of them will.
> I'm not sure about what we should do for lpc32xx. Is that
> generic glue going into v3.4?
That's up to Greg. At the moment it isn't used by anything, and unless
an existing driver can be converted over I'd guess it's not likely to
get merged right away.
> If so, we could convert the
> pnx4008 driver to use that and abstract it in a way that works
> nicely for pnx4008 and lpc32xx. OTOH, we might just let this
> one go in as before and ask people to do it right for the next
> one.
I don't know anything about these two platforms. If they are
sufficiently similar then it does make sense to combine the drivers
into a single file, with various platform-specific decisions made at
runtime. These decisions don't necessarily have to use a machine_is_*
kind of test (although they can); it might be simpler to do such a
test once during probe and store the result in a flag for later reuse.
Or then again, it might not since there's no obvious place to keep
such a flag...
At the moment, switching the pnx4008 driver to use the generic bus glue
doesn't look easy. The generic code doesn't know anything about i2c.
Alan Stern
On Sunday 26 February 2012, Alan Stern wrote:
> On Sun, 26 Feb 2012, Arnd Bergmann wrote:
>
> > We are doing a major rework of the ARM architecture tree right now
> > to allow building multiple SoC platforms together, which has not
> > been possible traditionally. The "PLATFORM_DRIVER" macro in ohci
> > and ehci is one of many things standing in the way still and will have
> > to be dealt with at some point.
>
> Is there anybody in particular who is going to attack this issue?
Not the USB portion, we're still busy with more fundamental issues like
conflicting header files between the various subarchitectures.
> > > A little progress has been made already. We just received a submission
> > > for a "generic" platform bus glue file that will be able to take over
> > > the jobs of several of the existiing files. If anyone wants to take
> > > this further I won't object, but I don't plan to work on it myself
> > > soon.
> >
> > Ok, good to know. The approach of a generic glue is exactly what I
> > was going to suggest. As we get to ARM platforms that are currently
> > using PLATFORM_DRIVER, I will then ask people to convert the ohci
> > glue to use that generic glue instead of adding another *_DRIVER
> > macro to ohci/ehci.
>
> The problem is that several platforms have highly specialized needs
> that can't sanely be handled in a single generic driver. Although a
> lot of drivers can be converted over, not all of them will.
Can you give an example? We have recently converted the sdhci mmc drivers
in a similar way, and we have the libata drivers that have always worked
in this manner.
> > I'm not sure about what we should do for lpc32xx. Is that
> > generic glue going into v3.4?
>
> That's up to Greg. At the moment it isn't used by anything, and unless
> an existing driver can be converted over I'd guess it's not likely to
> get merged right away.
Where is that patch? Maybe I can help out by converting the PCI
variants of ohci and ehci to the generic glue, because they are
easy to test.
> > If so, we could convert the
> > pnx4008 driver to use that and abstract it in a way that works
> > nicely for pnx4008 and lpc32xx. OTOH, we might just let this
> > one go in as before and ask people to do it right for the next
> > one.
>
> I don't know anything about these two platforms. If they are
> sufficiently similar then it does make sense to combine the drivers
> into a single file, with various platform-specific decisions made at
> runtime. These decisions don't necessarily have to use a machine_is_*
> kind of test (although they can); it might be simpler to do such a
> test once during probe and store the result in a flag for later reuse.
> Or then again, it might not since there's no obvious place to keep
> such a flag...
>
> At the moment, switching the pnx4008 driver to use the generic bus glue
> doesn't look easy. The generic code doesn't know anything about i2c.
Maybe I misunderstood what the generic bus glue does then, because I
would not expect that it should know about i2c ;-)
I would think the generic bus glue would be a very simple library that
just exports the various symbols that are defined in ohci-hcd.c and
used in the bus specific driver so that the driver can be a separate
module. Is it something different from that?
Arnd
On Mon, 27 Feb 2012, Arnd Bergmann wrote:
> > > > A little progress has been made already. We just received a submission
> > > > for a "generic" platform bus glue file that will be able to take over
> > > > the jobs of several of the existiing files. If anyone wants to take
> > > > this further I won't object, but I don't plan to work on it myself
> > > > soon.
> > >
> > > Ok, good to know. The approach of a generic glue is exactly what I
> > > was going to suggest. As we get to ARM platforms that are currently
> > > using PLATFORM_DRIVER, I will then ask people to convert the ohci
> > > glue to use that generic glue instead of adding another *_DRIVER
> > > macro to ohci/ehci.
> >
> > The problem is that several platforms have highly specialized needs
> > that can't sanely be handled in a single generic driver. Although a
> > lot of drivers can be converted over, not all of them will.
>
> Can you give an example? We have recently converted the sdhci mmc drivers
> in a similar way, and we have the libata drivers that have always worked
> in this manner.
Look at ehci-tegra.c. It has numerous non-generic overrides, such as
tegra_ehci_hub_control(). It also has support for two clocks and a
transceiver.
> > > I'm not sure about what we should do for lpc32xx. Is that
> > > generic glue going into v3.4?
> >
> > That's up to Greg. At the moment it isn't used by anything, and unless
> > an existing driver can be converted over I'd guess it's not likely to
> > get merged right away.
>
> Where is that patch? Maybe I can help out by converting the PCI
> variants of ohci and ehci to the generic glue, because they are
> easy to test.
http://marc.info/?l=linux-usb&m=132986573930143&w=2
and the following two messages in the linux-usb mailing list.
> > At the moment, switching the pnx4008 driver to use the generic bus glue
> > doesn't look easy. The generic code doesn't know anything about i2c.
>
> Maybe I misunderstood what the generic bus glue does then, because I
> would not expect that it should know about i2c ;-)
>
> I would think the generic bus glue would be a very simple library that
> just exports the various symbols that are defined in ohci-hcd.c and
> used in the bus specific driver so that the driver can be a separate
> module. Is it something different from that?
Quite different. What you just described is ohci-hcd.c itself --
except that it doesn't export the necessary symbols.
The generic driver, as you'll see when you read the patch, includes
generic versions of the various routines that a bus glue file has to
provide (probe, remove, initialize, and so on).
Alan Stern
On Monday 27 February 2012, Alan Stern wrote:
> On Mon, 27 Feb 2012, Arnd Bergmann wrote:
>
> > > At the moment, switching the pnx4008 driver to use the generic bus glue
> > > doesn't look easy. The generic code doesn't know anything about i2c.
> >
> > Maybe I misunderstood what the generic bus glue does then, because I
> > would not expect that it should know about i2c ;-)
> >
> > I would think the generic bus glue would be a very simple library that
> > just exports the various symbols that are defined in ohci-hcd.c and
> > used in the bus specific driver so that the driver can be a separate
> > module. Is it something different from that?
>
> Quite different. What you just described is ohci-hcd.c itself --
> except that it doesn't export the necessary symbols.
>
> The generic driver, as you'll see when you read the patch, includes
> generic versions of the various routines that a bus glue file has to
> provide (probe, remove, initialize, and so on).
Ok, I see. Nevermind then, I don't think this will help to solve
the problem of building multiple ARM platforms together when they
provide conflicting bus glues, although it seems generally to be
a good idea in order to reduce the number of platform glues that there
are
What do you think about an approach like below?
Arnd
8<----------
[POC] usb/ohci: Turn ohci-hcd.c into a library
This is an attempt to turn the ohci host controller driver into a library
similar to how many other kernel drivers work that have a common register
interface on a number of different buses.
The conversion is straightforward: all functions that are used by the pci
specific frontend are exported from the common code and declared in a header.
The ohci-pci.c file is turned into a module by listing it in the Makefile
and adding a module_pci_driver() definition as well as all the #include
statements that are needed to build it standalone.
Doing the same for the other bus glues should be a trivial change to
enable separate compilation in each one, with a module_platform_driver()
statement.
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/usb/host/Makefile | 1 +
drivers/usb/host/ohci-dbg.c | 10 +---
drivers/usb/host/ohci-hcd.c | 88 +++++++++++------------------------------
drivers/usb/host/ohci-hub.c | 19 ++++++---
drivers/usb/host/ohci-mem.c | 3 +-
drivers/usb/host/ohci-pci.c | 47 +++++++++++++--------
drivers/usb/host/ohci-q.c | 14 ++++---
drivers/usb/host/ohci.h | 37 +++++++++++++++++
drivers/usb/host/pci-quirks.c | 13 ++++++
drivers/usb/host/pci-quirks.h | 2 +
10 files changed, 130 insertions(+), 104 deletions(-)
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index 7ca290f..338e1cc 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_USB_OXU210HP_HCD) += oxu210hp-hcd.o
obj-$(CONFIG_USB_ISP116X_HCD) += isp116x-hcd.o
obj-$(CONFIG_USB_ISP1362_HCD) += isp1362-hcd.o
obj-$(CONFIG_USB_OHCI_HCD) += ohci-hcd.o
+obj-$(CONFIG_USB_OHCI_HCD_PCI) += ohci-pci.o
obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o
obj-$(CONFIG_USB_FHCI_HCD) += fhci.o
obj-$(CONFIG_USB_XHCI_HCD) += xhci-hcd.o
diff --git a/drivers/usb/host/ohci-dbg.c b/drivers/usb/host/ohci-dbg.c
index e4bcb62..1b868b76 100644
--- a/drivers/usb/host/ohci-dbg.c
+++ b/drivers/usb/host/ohci-dbg.c
@@ -289,7 +289,7 @@ ohci_dump_roothub (
}
}
-static void ohci_dump (struct ohci_hcd *controller, int verbose)
+void ohci_dump (struct ohci_hcd *controller, int verbose)
{
ohci_dbg (controller, "OHCI controller state\n");
@@ -300,6 +300,7 @@ static void ohci_dump (struct ohci_hcd *controller, int verbose)
"hcca frame #%04x\n", ohci_frame_no(controller));
ohci_dump_roothub (controller, 1, NULL, NULL);
}
+EXPORT_SYMBOL_GPL(ohci_dump);
static const char data0 [] = "DATA0";
static const char data1 [] = "DATA1";
@@ -407,12 +408,7 @@ ohci_dump_ed (const struct ohci_hcd *ohci, const char *label,
}
}
-#else
-static inline void ohci_dump (struct ohci_hcd *controller, int verbose) {}
-
-#undef OHCI_VERBOSE_DEBUG
-
-#endif /* DEBUG */
+#endif
/*-------------------------------------------------------------------------*/
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 34b9edd..99c8dc4 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -72,31 +72,12 @@
/*-------------------------------------------------------------------------*/
-static const char hcd_name [] = "ohci_hcd";
+static const char hcd_name [] = "ohci_hcd";
#define STATECHANGE_DELAY msecs_to_jiffies(300)
#include "ohci.h"
#include "pci-quirks.h"
-
-static void ohci_dump (struct ohci_hcd *ohci, int verbose);
-static int ohci_init (struct ohci_hcd *ohci);
-static void ohci_stop (struct usb_hcd *hcd);
-
-#if defined(CONFIG_PM) || defined(CONFIG_PCI)
-static int ohci_restart (struct ohci_hcd *ohci);
-#endif
-
-#ifdef CONFIG_PCI
-static void sb800_prefetch(struct ohci_hcd *ohci, int on);
-#else
-static inline void sb800_prefetch(struct ohci_hcd *ohci, int on)
-{
- return;
-}
-#endif
-
-
#include "ohci-hub.c"
#include "ohci-dbg.c"
#include "ohci-mem.c"
@@ -130,7 +111,7 @@ MODULE_PARM_DESC (no_handshake, "true (not default) disables BIOS handshake");
/*
* queue up an urb for anything except the root hub
*/
-static int ohci_urb_enqueue (
+int ohci_urb_enqueue (
struct usb_hcd *hcd,
struct urb *urb,
gfp_t mem_flags
@@ -253,6 +234,7 @@ fail:
spin_unlock_irqrestore (&ohci->lock, flags);
return retval;
}
+EXPORT_SYMBOL_GPL(ohci_urb_enqueue);
/*
* decouple the URB from the HC queues (TDs, urb_priv).
@@ -260,7 +242,7 @@ fail:
* asynchronously, and we might be dealing with an urb that's
* partially transferred, or an ED with other urbs being unlinked.
*/
-static int ohci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
+int ohci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
{
struct ohci_hcd *ohci = hcd_to_ohci (hcd);
unsigned long flags;
@@ -297,6 +279,7 @@ static int ohci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
spin_unlock_irqrestore (&ohci->lock, flags);
return rc;
}
+EXPORT_SYMBOL_GPL(ohci_urb_dequeue);
/*-------------------------------------------------------------------------*/
@@ -304,8 +287,7 @@ static int ohci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
* including ED memory, dummy TD, and bulk/intr data toggle
*/
-static void
-ohci_endpoint_disable (struct usb_hcd *hcd, struct usb_host_endpoint *ep)
+void ohci_endpoint_disable(struct usb_hcd *hcd, struct usb_host_endpoint *ep)
{
struct ohci_hcd *ohci = hcd_to_ohci (hcd);
unsigned long flags;
@@ -364,28 +346,30 @@ sanitize:
ep->hcpriv = NULL;
spin_unlock_irqrestore (&ohci->lock, flags);
}
+EXPORT_SYMBOL_GPL(ohci_endpoint_disable);
-static int ohci_get_frame (struct usb_hcd *hcd)
+int ohci_get_frame(struct usb_hcd *hcd)
{
struct ohci_hcd *ohci = hcd_to_ohci (hcd);
return ohci_frame_no(ohci);
}
+EXPORT_SYMBOL_GPL(ohci_get_frame);
-static void ohci_usb_reset (struct ohci_hcd *ohci)
+void ohci_usb_reset(struct ohci_hcd *ohci)
{
ohci->hc_control = ohci_readl (ohci, &ohci->regs->control);
ohci->hc_control &= OHCI_CTRL_RWC;
ohci_writel (ohci, ohci->hc_control, &ohci->regs->control);
ohci->rh_state = OHCI_RH_HALTED;
}
+EXPORT_SYMBOL_GPL(ohci_usb_reset);
/* ohci_shutdown forcibly disables IRQs and DMA, helping kexec and
* other cases where the next software may expect clean state from the
* "firmware". this is bus-neutral, unlike shutdown() methods.
*/
-static void
-ohci_shutdown (struct usb_hcd *hcd)
+void ohci_shutdown(struct usb_hcd *hcd)
{
struct ohci_hcd *ohci;
@@ -399,6 +383,7 @@ ohci_shutdown (struct usb_hcd *hcd)
ohci_writel(ohci, ohci->fminterval, &ohci->regs->fminterval);
}
+EXPORT_SYMBOL_GPL(ohci_shutdown);
static int check_ed(struct ohci_hcd *ohci, struct ed *ed)
{
@@ -493,7 +478,7 @@ done:
/* init memory, and kick BIOS/SMM off */
-static int ohci_init (struct ohci_hcd *ohci)
+int ohci_init (struct ohci_hcd *ohci)
{
int ret;
struct usb_hcd *hcd = ohci_to_hcd(ohci);
@@ -563,6 +548,7 @@ static int ohci_init (struct ohci_hcd *ohci)
return ret;
}
+EXPORT_SYMBOL_GPL(ohci_init);
/*-------------------------------------------------------------------------*/
@@ -570,7 +556,7 @@ static int ohci_init (struct ohci_hcd *ohci)
* resets USB and controller
* enable interrupts
*/
-static int ohci_run (struct ohci_hcd *ohci)
+int ohci_run (struct ohci_hcd *ohci)
{
u32 mask, val;
int first = ohci->fminterval == 0;
@@ -740,12 +726,13 @@ retry:
return 0;
}
+EXPORT_SYMBOL_GPL(ohci_run);
/*-------------------------------------------------------------------------*/
/* an interrupt happens */
-static irqreturn_t ohci_irq (struct usb_hcd *hcd)
+irqreturn_t ohci_irq (struct usb_hcd *hcd)
{
struct ohci_hcd *ohci = hcd_to_ohci (hcd);
struct ohci_regs __iomem *regs = ohci->regs;
@@ -884,10 +871,11 @@ static irqreturn_t ohci_irq (struct usb_hcd *hcd)
return IRQ_HANDLED;
}
+EXPORT_SYMBOL_GPL(ohci_irq);
/*-------------------------------------------------------------------------*/
-static void ohci_stop (struct usb_hcd *hcd)
+void ohci_stop (struct usb_hcd *hcd)
{
struct ohci_hcd *ohci = hcd_to_ohci (hcd);
@@ -916,13 +904,14 @@ static void ohci_stop (struct usb_hcd *hcd)
ohci->hcca_dma = 0;
}
}
+EXPORT_SYMBOL_GPL(ohci_stop);
/*-------------------------------------------------------------------------*/
#if defined(CONFIG_PM) || defined(CONFIG_PCI)
/* must not be called from interrupt context */
-static int ohci_restart (struct ohci_hcd *ohci)
+int ohci_restart (struct ohci_hcd *ohci)
{
int temp;
int i;
@@ -981,7 +970,7 @@ static int ohci_restart (struct ohci_hcd *ohci)
ohci_dbg(ohci, "restart complete\n");
return 0;
}
-
+EXPORT_SYMBOL_GPL(ohci_restart);
#endif
/*-------------------------------------------------------------------------*/
@@ -990,11 +979,6 @@ MODULE_AUTHOR (DRIVER_AUTHOR);
MODULE_DESCRIPTION(DRIVER_DESC);
MODULE_LICENSE ("GPL");
-#ifdef CONFIG_PCI
-#include "ohci-pci.c"
-#define PCI_DRIVER ohci_pci_driver
-#endif
-
#if defined(CONFIG_ARCH_SA1100) && defined(CONFIG_SA1111)
#include "ohci-sa1111.c"
#define SA1111_DRIVER ohci_hcd_sa1111_driver
@@ -1121,19 +1105,6 @@ MODULE_LICENSE ("GPL");
#define PLATFORM_DRIVER ohci_xls_driver
#endif
-#if !defined(PCI_DRIVER) && \
- !defined(PLATFORM_DRIVER) && \
- !defined(OMAP1_PLATFORM_DRIVER) && \
- !defined(OMAP3_PLATFORM_DRIVER) && \
- !defined(OF_PLATFORM_DRIVER) && \
- !defined(SA1111_DRIVER) && \
- !defined(PS3_SYSTEM_BUS_DRIVER) && \
- !defined(SM501_OHCI_DRIVER) && \
- !defined(TMIO_OHCI_DRIVER) && \
- !defined(SSB_OHCI_DRIVER)
-#error "missing bus glue for ohci-hcd"
-#endif
-
static int __init ohci_hcd_mod_init(void)
{
int retval = 0;
@@ -1190,12 +1161,6 @@ static int __init ohci_hcd_mod_init(void)
goto error_sa1111;
#endif
-#ifdef PCI_DRIVER
- retval = pci_register_driver(&PCI_DRIVER);
- if (retval < 0)
- goto error_pci;
-#endif
-
#ifdef SSB_OHCI_DRIVER
retval = ssb_driver_register(&SSB_OHCI_DRIVER);
if (retval)
@@ -1229,10 +1194,6 @@ static int __init ohci_hcd_mod_init(void)
ssb_driver_unregister(&SSB_OHCI_DRIVER);
error_ssb:
#endif
-#ifdef PCI_DRIVER
- pci_unregister_driver(&PCI_DRIVER);
- error_pci:
-#endif
#ifdef SA1111_DRIVER
sa1111_driver_unregister(&SA1111_DRIVER);
error_sa1111:
@@ -1279,9 +1240,6 @@ static void __exit ohci_hcd_mod_exit(void)
#ifdef SSB_OHCI_DRIVER
ssb_driver_unregister(&SSB_OHCI_DRIVER);
#endif
-#ifdef PCI_DRIVER
- pci_unregister_driver(&PCI_DRIVER);
-#endif
#ifdef SA1111_DRIVER
sa1111_driver_unregister(&SA1111_DRIVER);
#endif
diff --git a/drivers/usb/host/ohci-hub.c b/drivers/usb/host/ohci-hub.c
index 836772d..b7c6c05 100644
--- a/drivers/usb/host/ohci-hub.c
+++ b/drivers/usb/host/ohci-hub.c
@@ -279,7 +279,7 @@ skip_resume:
return 0;
}
-static int ohci_bus_suspend (struct usb_hcd *hcd)
+int ohci_bus_suspend (struct usb_hcd *hcd)
{
struct ohci_hcd *ohci = hcd_to_ohci (hcd);
int rc;
@@ -293,8 +293,9 @@ static int ohci_bus_suspend (struct usb_hcd *hcd)
spin_unlock_irq (&ohci->lock);
return rc;
}
+EXPORT_SYMBOL_GPL(ohci_bus_suspend);
-static int ohci_bus_resume (struct usb_hcd *hcd)
+int ohci_bus_resume (struct usb_hcd *hcd)
{
struct ohci_hcd *ohci = hcd_to_ohci (hcd);
int rc;
@@ -315,9 +316,10 @@ static int ohci_bus_resume (struct usb_hcd *hcd)
usb_hcd_poll_rh_status(hcd);
return rc;
}
+EXPORT_SYMBOL_GPL(ohci_bus_resume);
/* Carry out the final steps of resuming the controller device */
-static void ohci_finish_controller_resume(struct usb_hcd *hcd)
+void ohci_finish_controller_resume(struct usb_hcd *hcd)
{
struct ohci_hcd *ohci = hcd_to_ohci(hcd);
int port;
@@ -357,6 +359,7 @@ static void ohci_finish_controller_resume(struct usb_hcd *hcd)
usb_hcd_resume_root_hub(hcd);
}
+EXPORT_SYMBOL_GPL(ohci_finish_controller_resume);
/* Carry out polling-, autostop-, and autoresume-related state changes */
static int ohci_root_hub_state_changes(struct ohci_hcd *ohci, int changed,
@@ -480,8 +483,7 @@ static int ohci_root_hub_state_changes(struct ohci_hcd *ohci, int changed,
/* build "status change" packet (one or two bytes) from HC registers */
-static int
-ohci_hub_status_data (struct usb_hcd *hcd, char *buf)
+int ohci_hub_status_data (struct usb_hcd *hcd, char *buf)
{
struct ohci_hcd *ohci = hcd_to_ohci (hcd);
int i, changed = 0, length = 1;
@@ -546,6 +548,7 @@ done:
return changed ? length : 0;
}
+EXPORT_SYMBOL_GPL(ohci_hub_status_data);
/*-------------------------------------------------------------------------*/
@@ -592,7 +595,7 @@ ohci_hub_descriptor (
#ifdef CONFIG_USB_OTG
-static int ohci_start_port_reset (struct usb_hcd *hcd, unsigned port)
+int ohci_start_port_reset (struct usb_hcd *hcd, unsigned port)
{
struct ohci_hcd *ohci = hcd_to_ohci (hcd);
u32 status;
@@ -610,6 +613,7 @@ static int ohci_start_port_reset (struct usb_hcd *hcd, unsigned port)
ohci_writel(ohci, RH_PS_PRS, &ohci->regs->roothub.portstatus [port]);
return 0;
}
+EXPORT_SYMBOL_GPL(ohci_start_port_reset);
#else
@@ -694,7 +698,7 @@ static inline int root_port_reset (struct ohci_hcd *ohci, unsigned port)
return 0;
}
-static int ohci_hub_control (
+int ohci_hub_control (
struct usb_hcd *hcd,
u16 typeReq,
u16 wValue,
@@ -822,4 +826,5 @@ error:
}
return retval;
}
+EXPORT_SYMBOL_GPL(ohci_hub_control);
diff --git a/drivers/usb/host/ohci-mem.c b/drivers/usb/host/ohci-mem.c
index 2f20d3d..1a4dede 100644
--- a/drivers/usb/host/ohci-mem.c
+++ b/drivers/usb/host/ohci-mem.c
@@ -23,12 +23,13 @@
/*-------------------------------------------------------------------------*/
-static void ohci_hcd_init (struct ohci_hcd *ohci)
+void ohci_hcd_init (struct ohci_hcd *ohci)
{
ohci->next_statechange = jiffies;
spin_lock_init (&ohci->lock);
INIT_LIST_HEAD (&ohci->pending);
}
+EXPORT_SYMBOL_GPL(ohci_hcd_init);
/*-------------------------------------------------------------------------*/
diff --git a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.c
index 1843bb6..fa436ea 100644
--- a/drivers/usb/host/ohci-pci.c
+++ b/drivers/usb/host/ohci-pci.c
@@ -14,13 +14,35 @@
* This file is licenced under the GPL.
*/
-#ifndef CONFIG_PCI
-#error "This file is PCI bus glue. CONFIG_PCI must be defined."
-#endif
-
+#include <linux/module.h>
+#include <linux/moduleparam.h>
#include <linux/pci.h>
-#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/sched.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/timer.h>
+#include <linux/usb.h>
+#include <linux/usb/otg.h>
+#include <linux/usb/hcd.h>
+#include <linux/workqueue.h>
+
+#include <asm/io.h>
+#include <asm/irq.h>
+#include <asm/system.h>
+#include <asm/unaligned.h>
+
+#include "ohci.h"
+#include "pci-quirks.h"
+
+#define DRIVER_AUTHOR "Roman Weissgaerber, David Brownell"
+#define DRIVER_DESC "USB 1.1 'Open' Host Controller (OHCI) Driver"
+static const char hcd_name [] = "ohci_hcd";
+
+MODULE_AUTHOR (DRIVER_AUTHOR);
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_LICENSE ("GPL");
/*-------------------------------------------------------------------------*/
@@ -175,19 +197,6 @@ static int ohci_quirk_amd700(struct usb_hcd *hcd)
return 0;
}
-static void sb800_prefetch(struct ohci_hcd *ohci, int on)
-{
- struct pci_dev *pdev;
- u16 misc;
-
- pdev = to_pci_dev(ohci_to_hcd(ohci)->self.controller);
- pci_read_config_word(pdev, 0x50, &misc);
- if (on == 0)
- pci_write_config_word(pdev, 0x50, misc & 0xfcff);
- else
- pci_write_config_word(pdev, 0x50, misc | 0x0300);
-}
-
/* List of quirks for OHCI */
static const struct pci_device_id ohci_pci_quirks[] = {
{
@@ -420,3 +429,5 @@ static struct pci_driver ohci_pci_driver = {
},
#endif
};
+
+module_pci_driver(ohci_pci_driver);
diff --git a/drivers/usb/host/ohci-q.c b/drivers/usb/host/ohci-q.c
index c5a1ea9..e7e86db 100644
--- a/drivers/usb/host/ohci-q.c
+++ b/drivers/usb/host/ohci-q.c
@@ -42,6 +42,7 @@ __releases(ohci->lock)
__acquires(ohci->lock)
{
// ASSERT (urb->hcpriv != 0);
+ struct usb_bus *bus = &ohci_to_hcd(ohci)->self;
urb_free_priv (ohci, urb->hcpriv);
if (likely(status == -EINPROGRESS))
@@ -49,16 +50,16 @@ __acquires(ohci->lock)
switch (usb_pipetype (urb->pipe)) {
case PIPE_ISOCHRONOUS:
- ohci_to_hcd(ohci)->self.bandwidth_isoc_reqs--;
- if (ohci_to_hcd(ohci)->self.bandwidth_isoc_reqs == 0) {
+ bus->bandwidth_isoc_reqs--;
+ if (bus->bandwidth_isoc_reqs == 0) {
if (quirk_amdiso(ohci))
usb_amd_quirk_pll_enable();
if (quirk_amdprefetch(ohci))
- sb800_prefetch(ohci, 0);
+ usb_sb800_prefetch(bus->controller, 0);
}
break;
case PIPE_INTERRUPT:
- ohci_to_hcd(ohci)->self.bandwidth_int_reqs--;
+ bus->bandwidth_int_reqs--;
break;
}
@@ -73,7 +74,7 @@ __acquires(ohci->lock)
spin_lock (&ohci->lock);
/* stop periodic dma if it's not needed */
- if (ohci_to_hcd(ohci)->self.bandwidth_isoc_reqs == 0
+ if (bus->bandwidth_isoc_reqs == 0
&& ohci_to_hcd(ohci)->self.bandwidth_int_reqs == 0) {
ohci->hc_control &= ~(OHCI_CTRL_PLE|OHCI_CTRL_IE);
ohci_writel (ohci, ohci->hc_control, &ohci->regs->control);
@@ -579,6 +580,7 @@ static void td_submit_urb (
struct urb *urb
) {
struct urb_priv *urb_priv = urb->hcpriv;
+ struct usb_bus *bus = &ohci_to_hcd(ohci)->self;
dma_addr_t data;
int data_len = urb->transfer_buffer_length;
int cnt = 0;
@@ -688,7 +690,7 @@ static void td_submit_urb (
if (quirk_amdiso(ohci))
usb_amd_quirk_pll_disable();
if (quirk_amdprefetch(ohci))
- sb800_prefetch(ohci, 1);
+ usb_sb800_prefetch(bus->controller, 1);
}
periodic = ohci_to_hcd(ohci)->self.bandwidth_isoc_reqs++ == 0
&& ohci_to_hcd(ohci)->self.bandwidth_int_reqs == 0;
diff --git a/drivers/usb/host/ohci.h b/drivers/usb/host/ohci.h
index 8ff6f7e..0c99267 100644
--- a/drivers/usb/host/ohci.h
+++ b/drivers/usb/host/ohci.h
@@ -723,3 +723,40 @@ static inline u32 roothub_status (struct ohci_hcd *hc)
{ return ohci_readl (hc, &hc->regs->roothub.status); }
static inline u32 roothub_portstatus (struct ohci_hcd *hc, int i)
{ return read_roothub (hc, portstatus [i], 0xffe0fce0); }
+
+#ifdef DEBUG
+void ohci_dump(struct ohci_hcd *ohci, int verbose);
+#else
+static inline void ohci_dump(struct ohci_hcd *controller, int verbose) {}
+#undef OHCI_VERBOSE_DEBUG
+#endif /* DEBUG */
+
+/* ohci-hub.c */
+int ohci_bus_suspend(struct usb_hcd *hcd);
+int ohci_bus_resume(struct usb_hcd *hcd);
+void ohci_finish_controller_resume(struct usb_hcd *hcd);
+int ohci_hub_status_data (struct usb_hcd *hcd, char *buf);
+int ohci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, u16 wIndex, char *buf, u16 wLength);
+#ifdef CONFIG_USB_OTG
+int ohci_start_port_reset (struct usb_hcd *hcd, unsigned port);
+#else
+#define ohci_start_port_reset NULL
+#endif
+
+/* ohci-mem.c */
+void ohci_hcd_init(struct ohci_hcd *ohci);
+
+/* ohci-hcd.c */
+int ohci_init(struct ohci_hcd *ohci);
+void ohci_stop(struct usb_hcd *hcd);
+int ohci_restart(struct ohci_hcd *ohci);
+int ohci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flags);
+int ohci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status);
+void ohci_endpoint_disable(struct usb_hcd *hcd, struct usb_host_endpoint *ep);
+int ohci_get_frame(struct usb_hcd *hcd);
+irqreturn_t ohci_irq (struct usb_hcd *hcd);
+int ohci_run (struct ohci_hcd *ohci);
+void ohci_usb_reset(struct ohci_hcd *ohci);
+void ohci_shutdown(struct usb_hcd *hcd);
+
+
diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 7732d69..72f2d7a 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -181,6 +181,19 @@ commit:
}
EXPORT_SYMBOL_GPL(usb_amd_find_chipset_info);
+void usb_sb800_prefetch(struct device *dev, int on)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+ u16 misc;
+
+ pci_read_config_word(pdev, 0x50, &misc);
+ if (on == 0)
+ pci_write_config_word(pdev, 0x50, misc & 0xfcff);
+ else
+ pci_write_config_word(pdev, 0x50, misc | 0x0300);
+}
+EXPORT_SYMBOL_GPL(usb_sb800_prefetch);
+
/*
* The hardware normally enables the A-link power management feature, which
* lets the system lower the power consumption in idle states.
diff --git a/drivers/usb/host/pci-quirks.h b/drivers/usb/host/pci-quirks.h
index b1002a8..c473b0b 100644
--- a/drivers/usb/host/pci-quirks.h
+++ b/drivers/usb/host/pci-quirks.h
@@ -8,12 +8,14 @@ int usb_amd_find_chipset_info(void);
void usb_amd_dev_put(void);
void usb_amd_quirk_pll_disable(void);
void usb_amd_quirk_pll_enable(void);
+void usb_sb800_prefetch(struct device *dev, int on);
bool usb_is_intel_switchable_xhci(struct pci_dev *pdev);
void usb_enable_xhci_ports(struct pci_dev *xhci_pdev);
#else
static inline void usb_amd_quirk_pll_disable(void) {}
static inline void usb_amd_quirk_pll_enable(void) {}
static inline void usb_amd_dev_put(void) {}
+static inline void usb_sb800_prefetch(struct device *dev, int on) {}
#endif /* CONFIG_PCI */
#endif /* __LINUX_USB_PCI_QUIRKS_H */
On Mon, 27 Feb 2012, Arnd Bergmann wrote:
> > The generic driver, as you'll see when you read the patch, includes
> > generic versions of the various routines that a bus glue file has to
> > provide (probe, remove, initialize, and so on).
>
> Ok, I see. Nevermind then, I don't think this will help to solve
> the problem of building multiple ARM platforms together when they
> provide conflicting bus glues, although it seems generally to be
> a good idea in order to reduce the number of platform glues that there
> are
>
> What do you think about an approach like below?
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. :-)
Alan Stern
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
On Tue, 28 Feb 2012, Arnd Bergmann wrote:
> 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
Let's see. ohci-hcd should get a new bus-agnostic ohci_start routine.
This would call ohci_run internally, so the bus-glue files wouldn't
need to know about it.
Similarly, ohci_init and ohci_hcd_init could perhaps become part of a
bus-agnostic ohci_reset routine. Both ohci_start and ohci_reset would
be stored as fields in the ohci_driver structure, so they wouldn't need
to be exported.
On the other hand, ohci_finish_controller_resume really does need to be
exported; I don't see any way around that.
> And then there are a few symbols that are only used by one or two
> drivers, possibly correctly or not:
>
> ohci_dump (spear)
That's for debugging. Okay to export, although I don't know that the
spear driver actually needs it.
> ohci_usb_reset (at91, pci)
Not sure about this one...
> ohci_shutdown (ps3)
This will be stored as the .shutdown field in ohci_driver.
> ohci_restart (pci)
This one appears to be present just to handle a quirk of the NEC chip.
As such it ought to be moved entirely into ohci-pci.c.
> ohci_hub_control (da8xx)
This will be stored in ohci_driver.
> 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.
It's almost bikeshedding...
I like the idea of exporting ohci_driver, because then its member
methods don't have to be exported. Instead of calling
ohci_hub_control() directly, a bus-glue file can override it and then
internally invoke (ohci_driver->hub_control)().
In theory we could do both: export ohci_driver _and_ export a routine
to overwrite the uninitialized fields of a different structure with the
corresponding fields from ohci_driver. On the other hand, that routine
would look pretty mind-numbing:
if (!drv->reset)
drv->reset = ohci_driver.reset;
if (!drv->start)
drv->start = ohci_driver.start;
if (!drv->stop)
drv->stop = ohci_Driver.stop;
...
My preference is to have the bus-glue files do an explicit structure
copy and then overwrite by hand the fields that need to be changed.
The number of overrides in each file will be pretty small.
Alan Stern