On Fri, 25 Apr 2008, Takashi Iwai wrote: > At Fri, 25 Apr 2008 09:35:47 +0200 (CEST), > Jaroslav Kysela wrote: > > > > On Fri, 25 Apr 2008, Takashi Iwai wrote: > > > > > > Sure. I applied the simple 'void *device_private_data' patch, because > > > > current usage request is really trivial. We can implement complex code to > > > > handle data for multiple "extra" devices on AC97 bus later. > > > > > > Actually, it's not "used" yet. The ucb1000 reads the data but no one > > > stores yet. And, if its usage request is trivial, we should use "int > > > > Yes, I hope that the appropriate initialization code will be added to SoC > > drivers, too. > > > > > irq" as in the original patch instead of void data and cast. > > > > But other SoC (or other) drivers might want to pass to extra devices on > > AC97 bus something different or more complex. Mark Brown already noted > > that. I would keep it as 'void *'. > > That's the very problem I've been trying to point out. > The void pointer is good if the same driver assigns and casts. But, > in this case, the allocator and the receiver are different. Thus, > there is no guarantee that the data type is what you want. OTOH, if > it's "int irq", this is crystal clear. > > So, in short: > > - if only one device needs such data, it should be a strong type like > "int irq" anyway -- no extra need to cast to void pointer > - if multiple devices need such a pass-away mechanism, then they can > crash because you have no data type check. The void pointer is > dangerous for multiple devices. I see. In this case, I would propose to add a 32-bit "magic" at the start of 'void *' data. How about this modification: diff -r e2ff47e8771b include/ac97_codec.h --- a/include/ac97_codec.h Fri Apr 25 08:29:05 2008 +0200 +++ b/include/ac97_codec.h Fri Apr 25 10:22:00 2008 +0200 @@ -407,6 +407,9 @@ #define AC97_RATES_MIC_ADC 4 #define AC97_RATES_SPDIF 5 +/* device private data magic number */ +#define AC97_PDEVMAGIC_IRQ 0x20495251 /* in ASCII: <space>IRQ */ + /* * */ @@ -545,6 +547,11 @@ static inline int ac97_can_spdif(struct static inline int ac97_can_spdif(struct snd_ac97 * ac97) { return (ac97->ext_id & AC97_EI_SPDIF) != 0; +} +static inline int ac97_check_pdevdata_magic(struct snd_ac97 * ac97, unsigned int magic) +{ + return (ac97->device_private_data && + *((unsigned int *)ac97->device_private_data) == magic); } /* functions */ diff -r e2ff47e8771b kernel/drivers/input/touchscreen/ucb1400_ts.c --- a/kernel/drivers/input/touchscreen/ucb1400_ts.c Fri Apr 25 08:29:05 2008 +0200 +++ b/kernel/drivers/input/touchscreen/ucb1400_ts.c Fri Apr 25 10:22:00 2008 +0200 @@ -492,14 +492,14 @@ static int ucb1400_ts_probe(struct devic goto err_free_devs; } - if (!ucb->ac97->device_private_data) { + if (!ac97_check_pdevdata_magic(usb->ac97, AC97_PDEVMAGIC_IRQ)) { error = ucb1400_detect_irq(ucb); if (error) { printk(KERN_ERR "UCB1400: IRQ probe failed\n"); goto err_free_devs; } } else { - ucb->irq = (int) ucb->ac97->device_private_data; + ucb->irq = ((int *) ucb->ac97->device_private_data)[1]; } error = request_irq(ucb->irq, ucb1400_hard_irq, IRQF_TRIGGER_RISING, Jaroslav ----- Jaroslav Kysela <perex@xxxxxxxx> Linux Kernel Sound Maintainer ALSA Project, Red Hat, Inc. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html