Mario Goebbels wrote:
> I've cleaned up the little hacking I've did here and there this week.
> The patch works against build 1015 (CDDL). Comments about code quality
> would be nice, since I'm jumping into cold water here in regards to
> writing C.
> +#define ASUS_VENDOR_ID 0x1043 // Is it?
It is.
> +/* Xonar specific */
> +#define XONAR_DX_FRONTDAC 0x9e
> +#define XONAR_DX_GPIO_OUTPUT 0x01
> ...
> +/* Xonar DX */
> +#define XONAR_DX_OUTPUT 0x0001
> +#define XONAR_ADDR_FRONT 0x9e
?
> + /* Card model */
> + int card_model;
I think you can use the model field for this purpose.
> + /* Wait for it to stop being busy */
> + while((INW(devc->osdev, TWO_WIRE_CTRL) & 0x1) && (count > 0))
> + ...
> MUTEX_ENTER_IRQDISABLE (devc->low_mutex, flags);
Waiting outside the mutex means that this functions isn't reentrant.
> + oss_udelay(100);
> +
> MUTEX_EXIT_IRQRESTORE (devc->low_mutex, flags);
Why is this wait inside the mutex?
I think it would be a better idea to wrap the entire function in a mutex
that doesn't disable interupts.
> + // Left justified PCM (DAC and 8788 support I2S, but doesn't work).
How does it not work?
> + /* SPI communication */
> + bVal |= 0x80;
SPI is actually enabled by clearing bit 6. Bit 7 enables SPI outputs 4
and 5.
> + /* Must set master clock. */
> + sDac |= 0x0010;
Please mention "256".
> + /* Enable Xonar output */
> + unsigned short output_enable;
> + switch(devc->card_model)
> + {
> + case SUBID_XONAR_DX:
> + output_enable = XONAR_DX_OUTPUT;
> + break;
> + }
> + OUTW(devc->osdev, output_enable, GPIO_CONTROL);
> + OUTW(devc->osdev, output_enable, GPIO_DATA);
output_enable is not initialized when the model is not a Xonar DX.
Regards,
Clemens
_______________________________________________
oss-devel mailing list
oss-devel@mailman.opensound.com
http://mailman.opensound.com/mailman/listinfo/oss-devel
