Sound: adding /proc/driver/{vendor}/{dev_pci}/ac97 entry for trident driver

PATCH In review salvador 2 participants 5 messages Jan 9, 2002 – Jan 11, 2002

lore

Share & tools

lore

salvador

author

2002-01-09 15:50 UTC #0 lore raw

PATCH 1/1 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)

drivers/sound/trident.c Tue Jan 8 21:41:05 2002 renamed from drivers/sound/trident.c Tue Nov 13 14:19:41 2001 +4 -1

@@ -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

Alan Cox

2002-01-09 16:09 UTC #1 lore raw

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

Review

Quoted 10 lines

(quoted diff) +2 -0
+               if (proc_mkdir (proc_str, 0)) {
+                       sprintf(proc_str, "driver/%s/%s", name,
card->pci_dev->slot_name);
(quoted diff) +2 -0
+                       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);
(quoted diff) +2 -0
+                       }
+               }

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

salvador

author

2002-01-09 17:39 UTC #2 lore raw

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

Quoted 11 lines · Alan Cox

Review
Quoted 10 lines
(quoted diff) +2 -0
+               if (proc_mkdir (proc_str, 0)) {
+                       sprintf(proc_str, "driver/%s/%s", name,
card->pci_dev->slot_name);
(quoted diff) +2 -0
+                       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);
(quoted diff) +2 -0
+                       }
+               }

Quoted 1 line

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?

Quoted 1 line

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

Alan Cox

2002-01-10 02:12 UTC #3 lore raw

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

Quoted 4 lines

Quoted 1 line
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)

salvador

author

2002-01-11 15:23 UTC #4 lore raw

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

Quoted 6 lines · Alan Cox

Quoted 4 lines
Quoted 1 line
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