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

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

[salvador]

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) <salvador@inti.gov.ar>
+ *     added a /proc/driver/{vendor}/{card}/ac97 entry
  *  v0.14.9d
  *     October 8 2001 Arnaldo Carvalho de Melo <acme@conectiva.com.br>
  *     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: set@computer.org set@ieee.org
Address: Curapaligue 2124, Caseros, 3 de Febrero
Buenos Aires, (1678), ARGENTINA Phone: +(5411) 4759 0013

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

[Alan Cox]

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

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

[salvador]

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: set@computer.org set@ieee.org
Address: Curapaligue 2124, Caseros, 3 de Febrero
Buenos Aires, (1678), ARGENTINA Phone: +(5411) 4759 0013

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

[Alan Cox]

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

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

[salvador]

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:      sailer@ife.ee.ethz.ch
L:      linux-sound@vger.kernel.org
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: set@computer.org set@ieee.org
Address: Curapaligue 2124, Caseros, 3 de Febrero
Buenos Aires, (1678), ARGENTINA Phone: +(5411) 4759 0013

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

[salvador]

William Robison wrote:

>   Alan mentioned that some cards make use of multiple codecs,
> but also consider those of us that have multiple sound
> cards when forming the '/proc' pathname.  As a quick&dirty
> hack to allow two 1371 cards to co-exist in /proc, I added
> the upper 8 bits of the address into the filename...
>
>   (One sound card dedicated to being used as a 1200 baud
>    modem for AX25 work, leaving one free for use as a
>    normal soundcard)

My patch creates an entry that looks like it:

/proc/drivers/es1371/00:03.00/ac97

Where 00:03.00 is PCI bus == 00, 03 is card 3 of the bus and the last 00 is
feature/chip on the card == 00.
This name is the unique name of the card created by the pci part of the
kernel. Somebody pointed out that this will change to allow more than 256
elements, in this case the id will change and the module will create a
correct unique name.
If you have 2 boards in the same PCI bus it will look like it:

/proc/drivers/es1371/00:03.00/ac97
/proc/drivers/es1371/00:04.00/ac97

If you have a board with a chip that implements 2 ESS1371/CT5880/... will
look like it:

/proc/drivers/es1371/00:03.00/ac97
/proc/drivers/es1371/00:03.01/ac97

And if you have 2 boards in different PCI buses:

/proc/drivers/es1371/00:03.00/ac97
/proc/drivers/es1371/01:03.00/ac97

These are just examples but I think you get the idea and understand that
there is no problem. What Alan pointed out (and he is 100% right) is that the
same DC97 chip (ESS1371 for example) can have attached more than one AC97
codec (upto 4 according the spec) all using the AC-link communication. In
this case the PCI bus, card number and feature number is the same for all the
codecs. I'm resending a patch that will create it:

/proc/drivers/es1371/00:03.00/ac97-0
/proc/drivers/es1371/00:03.00/ac97-1

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: set@computer.org set@ieee.org
Address: Curapaligue 2124, Caseros, 3 de Febrero
Buenos Aires, (1678), ARGENTINA Phone: +(5411) 4759 0013

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

[William Robison]

  Alan mentioned that some cards make use of multiple codecs,
but also consider those of us that have multiple sound
cards when forming the '/proc' pathname.  As a quick&dirty
hack to allow two 1371 cards to co-exist in /proc, I added
the upper 8 bits of the address into the filename...

  (One sound card dedicated to being used as a 1200 baud
   modem for AX25 work, leaving one free for use as a
   normal soundcard)

regards
-Willy
William Robison