2002-01-09 15:50:06

by Salvador Eduardo Tropea

[permalink] [raw]
Subject: [PATCH][RFCA] Sound: adding /proc/driver/{vendor}/{dev_pci}/ac97 entry for trident driver

Kernel: 2.4.17
Hardware: ALiM5451+ALC100P
Module: sound/trident.c

The following patch adds a proc entry to fetch information about the codec
attached to the audio "accelerator".
Note: Most of the code is already in ac97_codec.c module.

Doubts:
1) This adds something like: /proc/driver/ali5451/00:03.00/ac97 I took the
idea from emu10k1.c module. My doubt is about the `:' it looks wrong in a
file name, specially when that's usually used to separate file names/paths.
Should we pass it by a small routine that converts : into something like _?
2) I used a buffer of fixed length as in other modules, but I don't feel
really good doing it. What solutions are recommended? (if any)


--- linux-2.4.17.ori/drivers/sound/trident.c Tue Nov 13 14:19:41 2001
+++ linux-2.4.17/drivers/sound/trident.c Tue Jan 8 21:41:05 2002
@@ -36,6 +36,9 @@
* Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
*
* History
+ * v0.14.9e
+ * January 8 2002 Salvador E. Tropea (SET) <[email protected]>
+ * added a /proc/driver/{vendor}/{card}/ac97 entry
* v0.14.9d
* October 8 2001 Arnaldo Carvalho de Melo <[email protected]>
* use set_current_state, properly release resources on failure in
@@ -3829,8 +3832,8 @@
unsigned long ready_2nd = 0;
struct ac97_codec *codec;
int i = 0;
+ char proc_str[80], *name="unknown";

-
/* initialize controller side of AC link, and find out if secondary
codes
really exist */
switch (card->pci_id)
@@ -3853,6 +3856,7 @@
ready_2nd &= SI_AC97_SECONDARY_READY;
if (card->revision < ALI_5451_V02)
ready_2nd = 0;
+ name = "ali5451";
break;
case PCI_DEVICE_ID_SI_7018:
/* disable AC97 GPIO interrupt */
@@ -3865,16 +3869,19 @@
udelay(2000);
ready_2nd = inl(TRID_REG(card, SI_SERIAL_INTF_CTRL));
ready_2nd &= SI_AC97_SECONDARY_READY;
+ name = "sis7018";
break;
case PCI_DEVICE_ID_TRIDENT_4DWAVE_DX:
/* playback on */
outl(DX_AC97_PLAYBACK, TRID_REG(card,
DX_ACR2_AC97_COM_STAT));
+ name = "trident4D_DX";
break;
case PCI_DEVICE_ID_TRIDENT_4DWAVE_NX:
/* enable AC97 Output Slot 3,4 (PCM Left/Right Playback) */
outl(NX_AC97_PCM_OUTPUT, TRID_REG(card,
NX_ACR0_AC97_COM_STAT));
ready_2nd = inl(TRID_REG(card, NX_ACR0_AC97_COM_STAT));
ready_2nd &= NX_AC97_SECONDARY_READY;
+ name = "trident4D_NX";
break;
case PCI_DEVICE_ID_INTERG_5050:
/* disable AC97 GPIO interrupt */
@@ -3887,6 +3894,7 @@
udelay(2000);
ready_2nd = inl(TRID_REG(card, SI_SERIAL_INTF_CTRL));
ready_2nd &= SI_AC97_SECONDARY_READY;
+ name = "cyber5050";
break;
}

@@ -3919,6 +3927,16 @@
}

card->ac97_codec[num_ac97] = codec;
+
+ /* Create a proc entry to inform about the codec facilities
*/
+ sprintf(proc_str, "driver/%s", name);
+ if (proc_mkdir (proc_str, 0)) {
+ sprintf(proc_str, "driver/%s/%s", name,
card->pci_dev->slot_name);
+ if (proc_mkdir (proc_str, 0)) {
+ sprintf(proc_str, "driver/%s/%s/ac97", name,
card->pci_dev->slot_name);
+ create_proc_read_entry (proc_str, 0, 0,
ac97_read_proc, codec);
+ }
+ }

/* if there is no secondary codec at all, don't probe any
more */
if (!ready_2nd)
<---------End of patch

--
Salvador Eduardo Tropea (SET). (Electronics Engineer)
Visit my home page: http://welcome.to/SetSoft or
http://www.geocities.com/SiliconValley/Vista/6552/
Alternative e-mail: [email protected] [email protected]
Address: Curapaligue 2124, Caseros, 3 de Febrero
Buenos Aires, (1678), ARGENTINA Phone: +(5411) 4759 0013




2002-01-09 16:09:06

by Alan

[permalink] [raw]
Subject: Re: [PATCH][RFCA] Sound: adding /proc/driver/{vendor}/{dev_pci}/ac97 entry

> + if (proc_mkdir (proc_str, 0)) {
> + sprintf(proc_str, "driver/%s/%s", name,
> card->pci_dev->slot_name);
> + if (proc_mkdir (proc_str, 0)) {
> + sprintf(proc_str, "driver/%s/%s/ac97", name,
> card->pci_dev->slot_name);
> + create_proc_read_entry (proc_str, 0, 0,
> ac97_read_proc, codec);
> + }
> + }

Where do you remove it. Also a card can have multiple ac97 codecs

2002-01-09 17:39:06

by Salvador Eduardo Tropea

[permalink] [raw]
Subject: Re: [PATCH][RFCA] Sound: adding /proc/driver/{vendor}/{dev_pci}/ac97 entry

Alan Cox wrote:

> > + if (proc_mkdir (proc_str, 0)) {
> > + sprintf(proc_str, "driver/%s/%s", name,
> > card->pci_dev->slot_name);
> > + if (proc_mkdir (proc_str, 0)) {
> > + sprintf(proc_str, "driver/%s/%s/ac97", name,
> > card->pci_dev->slot_name);
> > + create_proc_read_entry (proc_str, 0, 0,
> > ac97_read_proc, codec);
> > + }
> > + }
>
> Where do you remove it.

I don't remove it, sorry I'll add it in the next iteration of the patch
BTW: I think the entry goes away when the module is removed. Is the kernel doing
a cleanup?

> Also a card can have multiple ac97 codecs

You are right, will also take care about it. Do you think
/proc/driver/{vendor}/{dev_pci}/{num_ac97}/ac97 will be ok?

SET

--
Salvador Eduardo Tropea (SET). (Electronics Engineer)
Visit my home page: http://welcome.to/SetSoft or
http://www.geocities.com/SiliconValley/Vista/6552/
Alternative e-mail: [email protected] [email protected]
Address: Curapaligue 2124, Caseros, 3 de Febrero
Buenos Aires, (1678), ARGENTINA Phone: +(5411) 4759 0013



2002-01-10 02:12:16

by Alan

[permalink] [raw]
Subject: Re: [PATCH][RFCA] Sound: adding /proc/driver/{vendor}/{dev_pci}/ac97

> > Also a card can have multiple ac97 codecs
>
> You are right, will also take care about it. Do you think
> /proc/driver/{vendor}/{dev_pci}/{num_ac97}/ac97 will be ok?

I'm dubious about the entire /proc entries. 8)

2002-01-11 15:23:21

by Salvador Eduardo Tropea

[permalink] [raw]
Subject: Re: [PATCH][RFCA] Sound: adding /proc/driver/{vendor}/{dev_pci}/ac97

Alan Cox wrote:

> > > Also a card can have multiple ac97 codecs
> >
> > You are right, will also take care about it. Do you think
> > /proc/driver/{vendor}/{dev_pci}/{num_ac97}/ac97 will be ok?
>
> I'm dubious about the entire /proc entries. 8)

I implemented it like it /proc/driver/{vendor}/{dev_pci}/ac-97-{num_ac97}
in the second revision of the patch.
I want to know if the second revision is acceptable.
I found a bug when using more than 1 board (duplicated entries in
/proc/driver) and fixed it so I'm creating a third revision. I also moved a
debug /proc entry created by es1371 module to the new directory because the
current behavior doesn't allow to debug more than one board. I tested it
using 2 boards (it was pointed out by William Robinson). I also moved an
entry created for ALi boards to the new directory (is cleaner and nobody
commented about it), also renamed it with a descriptive name. This entry is
used to enable the SPDIF part of the codec so now it is:
/proc/driver/ali5451/{dev_pci}/spdif
Also allowing for more than one ALiM5451 in the same board (really bizarre
since this is usually part of the chipset but .... how knows?)

SET

P.S. I didn't get any reply from the es1371 maintainer, is he actively
maintaining the module? The status indicates he is active so I think I must
be patient:

PCI SOUND DRIVERS (ES1370, ES1371 and SONICVIBES)
P: Thomas Sailer
M: [email protected]
L: [email protected]
W: http://www.ife.ee.ethz.ch/~sailer/linux/pciaudio.html
S: Maintained

--
Salvador Eduardo Tropea (SET). (Electronics Engineer)
Visit my home page: http://welcome.to/SetSoft or
http://www.geocities.com/SiliconValley/Vista/6552/
Alternative e-mail: [email protected] [email protected]
Address: Curapaligue 2124, Caseros, 3 de Febrero
Buenos Aires, (1678), ARGENTINA Phone: +(5411) 4759 0013