* [RFC] add a "private" field to the ac97 struct @ 2008-03-06 16:42 Sebastian Siewior 2008-03-06 17:12 ` William Pitcock 0 siblings, 1 reply; 4+ messages in thread From: Sebastian Siewior @ 2008-03-06 16:42 UTC (permalink / raw) To: Jaroslav Kysela; +Cc: alsa-devel, Nicolas Pitre I have a PowerPC board with an AC97 controller and that one has an UCB1400 attached. Currently the UCB1400 driver discovers the interrupt via probing. This doesn't really work here. The resources are read from the device tree. I haven't found any other driver besides ucb1400_ts that uses the ac97_bus_type so I can't check where others get their HW information from. My proposal is to introduce a new field in struct snd_ac97 where the AC97 driver can leave the required HW info regarding the attached (in my case the interrupt for the ucb1400) before calling snd_card_register(). I attached just a single int field since I am not aware if multiple devices / resource are realistic / possible. In that case maybe a void * or struct platform_device would be a better choice. Signed-off-by: Sebastian Siewior <sebastian@breakpoint.cc> --- drivers/input/touchscreen/ucb1400_ts.c | 13 ++++++++----- include/sound/ac97_codec.h | 2 ++ 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/drivers/input/touchscreen/ucb1400_ts.c b/drivers/input/touchscreen/ucb1400_ts.c index 607f993..95d1fea 100644 --- a/drivers/input/touchscreen/ucb1400_ts.c +++ b/drivers/input/touchscreen/ucb1400_ts.c @@ -492,11 +492,14 @@ static int ucb1400_ts_probe(struct device *dev) goto err_free_devs; } - error = ucb1400_detect_irq(ucb); - if (error) { - printk(KERN_ERR "UCB1400: IRQ probe failed\n"); - goto err_free_devs; - } + if (!ucb->ac97->device_irq) { + error = ucb1400_detect_irq(ucb); + if (error) { + printk(KERN_ERR "UCB1400: IRQ probe failed\n"); + goto err_free_devs; + } + } else + ucb->irq = ucb->ac97->device_irq; error = request_irq(ucb->irq, ucb1400_hard_irq, IRQF_TRIGGER_RISING, "UCB1400", ucb); diff --git a/include/sound/ac97_codec.h b/include/sound/ac97_codec.h index 0148058..bf920a2 100644 --- a/include/sound/ac97_codec.h +++ b/include/sound/ac97_codec.h @@ -474,6 +474,8 @@ struct snd_ac97 { struct snd_ac97_build_ops * build_ops; void *private_data; void (*private_free) (struct snd_ac97 *ac97); + /* device attached to the AC97 bus */ + int device_irq; /* --- */ struct snd_ac97_bus *bus; struct pci_dev *pci; /* assigned PCI device - used for quirks */ -- 1.5.3.5 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [RFC] add a "private" field to the ac97 struct 2008-03-06 16:42 [RFC] add a "private" field to the ac97 struct Sebastian Siewior @ 2008-03-06 17:12 ` William Pitcock 2008-03-06 17:22 ` Sebastian Siewior 0 siblings, 1 reply; 4+ messages in thread From: William Pitcock @ 2008-03-06 17:12 UTC (permalink / raw) To: Sebastian Siewior; +Cc: alsa-devel, Nicolas Pitre [-- Attachment #1.1: Type: text/plain, Size: 2639 bytes --] Hi, Why not modify ucb1400_detect_irq() to get an openfirmware handle on PowerPC and read the resource tree at that time? This seems like a better approach. William On Thu, 2008-03-06 at 17:42 +0100, Sebastian Siewior wrote: > I have a PowerPC board with an AC97 controller and that one has an > UCB1400 attached. > Currently the UCB1400 driver discovers the interrupt via probing. This > doesn't really work here. The resources are read from the device tree. > I haven't found any other driver besides ucb1400_ts that uses the > ac97_bus_type so I can't check where others get their HW information > from. > > My proposal is to introduce a new field in struct snd_ac97 where the > AC97 driver can leave the required HW info regarding the attached (in my > case the interrupt for the ucb1400) before calling snd_card_register(). > I attached just a single int field since I am not aware if multiple > devices / resource are realistic / possible. In that case maybe a void * > or struct platform_device would be a better choice. > > Signed-off-by: Sebastian Siewior <sebastian@breakpoint.cc> > --- > drivers/input/touchscreen/ucb1400_ts.c | 13 ++++++++----- > include/sound/ac97_codec.h | 2 ++ > 2 files changed, 10 insertions(+), 5 deletions(-) > > diff --git a/drivers/input/touchscreen/ucb1400_ts.c b/drivers/input/touchscreen/ucb1400_ts.c > index 607f993..95d1fea 100644 > --- a/drivers/input/touchscreen/ucb1400_ts.c > +++ b/drivers/input/touchscreen/ucb1400_ts.c > @@ -492,11 +492,14 @@ static int ucb1400_ts_probe(struct device *dev) > goto err_free_devs; > } > > - error = ucb1400_detect_irq(ucb); > - if (error) { > - printk(KERN_ERR "UCB1400: IRQ probe failed\n"); > - goto err_free_devs; > - } > + if (!ucb->ac97->device_irq) { > + error = ucb1400_detect_irq(ucb); > + if (error) { > + printk(KERN_ERR "UCB1400: IRQ probe failed\n"); > + goto err_free_devs; > + } > + } else > + ucb->irq = ucb->ac97->device_irq; > > error = request_irq(ucb->irq, ucb1400_hard_irq, IRQF_TRIGGER_RISING, > "UCB1400", ucb); > diff --git a/include/sound/ac97_codec.h b/include/sound/ac97_codec.h > index 0148058..bf920a2 100644 > --- a/include/sound/ac97_codec.h > +++ b/include/sound/ac97_codec.h > @@ -474,6 +474,8 @@ struct snd_ac97 { > struct snd_ac97_build_ops * build_ops; > void *private_data; > void (*private_free) (struct snd_ac97 *ac97); > + /* device attached to the AC97 bus */ > + int device_irq; > /* --- */ > struct snd_ac97_bus *bus; > struct pci_dev *pci; /* assigned PCI device - used for quirks */ [-- Attachment #1.2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 189 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC] add a "private" field to the ac97 struct 2008-03-06 17:12 ` William Pitcock @ 2008-03-06 17:22 ` Sebastian Siewior 2008-03-06 20:53 ` Mark Brown 0 siblings, 1 reply; 4+ messages in thread From: Sebastian Siewior @ 2008-03-06 17:22 UTC (permalink / raw) To: William Pitcock; +Cc: alsa-devel, Nicolas Pitre * William Pitcock | 2008-03-06 11:12:37 [-0600]: >Hi, Hi, >Why not modify ucb1400_detect_irq() to get an openfirmware handle on >PowerPC and read the resource tree at that time? This seems like a >better approach. That was my first approach: --- drivers/input/touchscreen/Kconfig | 7 ++++ drivers/input/touchscreen/ucb1400_ts.c | 57 +++++++++++++++++++++++++++++++- 2 files changed, 63 insertions(+), 1 deletions(-) diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig index 90e8e92..8f4d752 100644 --- a/drivers/input/touchscreen/Kconfig +++ b/drivers/input/touchscreen/Kconfig @@ -185,6 +185,13 @@ config TOUCHSCREEN_UCB1400 To compile this driver as a module, choose M here: the module will be called ucb1400_ts. +config TOUCHSCREEN_UCB1400_OF + bool "Get device informations via the device tree" + depends on TOUCHSCREEN_UCB1400 && OF + help + Select this option if you want to obtain interrupt information from + the device tree instead of auto probing. + config TOUCHSCREEN_USB_COMPOSITE tristate "USB Touchscreen Driver" depends on USB_ARCH_HAS_HCD diff --git a/drivers/input/touchscreen/ucb1400_ts.c b/drivers/input/touchscreen/ucb1400_ts.c index 607f993..9337352 100644 --- a/drivers/input/touchscreen/ucb1400_ts.c +++ b/drivers/input/touchscreen/ucb1400_ts.c @@ -26,6 +26,11 @@ #include <linux/kthread.h> #include <linux/freezer.h> +#ifdef CONFIG_TOUCHSCREEN_UCB1400_OF +#include <linux/of.h> +#include <linux/of_platform.h> +#endif + #include <sound/core.h> #include <sound/ac97_codec.h> @@ -418,6 +423,9 @@ static int ucb1400_ts_resume(struct device *dev) #define NO_IRQ 0 #endif +#ifdef CONFIG_TOUCHSCREEN_UCB1400_OF +static int ucb1400_irq; +#else /* * Try to probe our interrupt, rather than relying on lots of * hard-coded machine dependencies. @@ -467,6 +475,7 @@ static int ucb1400_detect_irq(struct ucb1400 *ucb) return 0; } +#endif static int ucb1400_ts_probe(struct device *dev) { @@ -491,12 +500,15 @@ static int ucb1400_ts_probe(struct device *dev) error = -ENODEV; goto err_free_devs; } - +#ifdef CONFIG_TOUCHSCREEN_UCB1400_OF + ucb->irq = ucb1400_irq; +#else error = ucb1400_detect_irq(ucb); if (error) { printk(KERN_ERR "UCB1400: IRQ probe failed\n"); goto err_free_devs; } +#endif error = request_irq(ucb->irq, ucb1400_hard_irq, IRQF_TRIGGER_RISING, "UCB1400", ucb); @@ -562,14 +574,57 @@ static struct device_driver ucb1400_ts_driver = { .resume = ucb1400_ts_resume, }; +#ifdef CONFIG_TOUCHSCREEN_UCB1400_OF +static int __devinit of_ucb_probe(struct of_device *dev, + const struct of_device_id *match) +{ + struct device_node *dp = dev->node; + + ucb1400_irq = irq_of_parse_and_map(dp, 0); + if (ucb1400_irq == IRQ_NONE) + return -ENODEV; + + return driver_register(&ucb1400_ts_driver); +} + +static int of_ucb_remove(struct of_device *dev) +{ + driver_unregister(&ucb1400_ts_driver); + return 0; +} + +static struct of_device_id of_ucb_match[] = { + { + .compatible = "Phillips-ucb1400_ts", + }, + { }, +}; +MODULE_DEVICE_TABLE(of, of_ucb_match); + +static struct of_platform_driver of_ucb1400_ts_driver = { + .name = "of-ucb1400_ts", + .match_table = of_ucb_match, + .probe = of_ucb_probe, + .remove = of_ucb_remove, +}; +#endif + static int __init ucb1400_ts_init(void) { +#ifdef CONFIG_TOUCHSCREEN_UCB1400_OF + return of_register_platform_driver(&of_ucb1400_ts_driver); +#else return driver_register(&ucb1400_ts_driver); +#endif } static void __exit ucb1400_ts_exit(void) { +#ifdef CONFIG_TOUCHSCREEN_UCB1400_OF + of_unregister_platform_driver(&of_ucb1400_ts_driver); +#else driver_unregister(&ucb1400_ts_driver); +#endif } module_param(adcsync, bool, 0444); -- 1.5.3.5 The problem is more or less that I can't pass the value to the other probe function and the global variable doesn't look all that pretty. Am I missing something here? >William Sebastian ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [RFC] add a "private" field to the ac97 struct 2008-03-06 17:22 ` Sebastian Siewior @ 2008-03-06 20:53 ` Mark Brown 0 siblings, 0 replies; 4+ messages in thread From: Mark Brown @ 2008-03-06 20:53 UTC (permalink / raw) To: Sebastian Siewior; +Cc: alsa-devel, Nicolas Pitre, William Pitcock On Thu, Mar 06, 2008 at 06:22:25PM +0100, Sebastian Siewior wrote: > The problem is more or less that I can't pass the value to the other > probe function and the global variable doesn't look all that pretty. Am > I missing something here? This is also a problem for the wm97xx touchscreen drivers. They have a similar issue with wanting to get platform configuration into a device probed off the AC97 bus with the additional issue that features like syncing with screen redraw will often require code to interface with other hardware in the system. If something is added to the AC97 data it'd be helpful if it were a void * so the wm97xx touch driver could use it to pass a struct with more data. This is not really an AC97-specific issue - any device discoverd via a bus is going to have a similar thing if it needs to obtain platform data. I don't know how OpenFirmware normally handles these things? It's going to be an issue for other buses like PCI too in many systems, MAC addresses for NICs being the example I've run into myself in a previous life. In the case of the wm97xx touchscreen we've avoided the problem by having the touch driver register platform devices; this isn't ideal but works well in the systems which are the typical targets for these devices. I had been intending to revisit this issue at some point but it's quite a way down my TODO list. The wm97xx drivers touch aren't in mainline yet - if you want to have a look the most current upstream submission versions can be found in the upstream branch of: git://opensource.wolfsonmicro.com/linux-2.6-touch gitweb: http://opensource.wolfsonmicro.com/cgi-bin/gitweb.cgi?p=linux-2.6-touch.git;a=summary ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2008-03-06 20:53 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-03-06 16:42 [RFC] add a "private" field to the ac97 struct Sebastian Siewior 2008-03-06 17:12 ` William Pitcock 2008-03-06 17:22 ` Sebastian Siewior 2008-03-06 20:53 ` Mark Brown
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.