Hi Frank, Frank Mandarino a écrit : > Mark Brown wrote: > >> The only major issue I see with the patch is a documentation one: it's >> not clear to me reading the code how the atmel_ssc DAI driver relates to >> the existing at91_ssc driver. It may be that this is something that's >> obvious to someone familiar with the at91 hardware but just looking at >> the code it's not clear to me what the difference is between the two and >> when each should be used. >> >> Looking at the code they appear to be similar to the point where they >> should be the same driver but it's entirely possible that I'm missing >> something here. I've CCed in Frank Mandarino who did the existing AT91 >> support. If they should be separate drivers then some comments should >> be added in the driver and the Kconfig help text explaning the >> situation. > > I agree that the drivers should be combined. Unfortunately, at this > time I am unable to contribute to this effort. > I agree with you as well. I wanted to use the drivers/misc/atmel-ssc in the dai because it is a common arch between atmel ARM and AVR core. I will have a look at the at91-ssc code and at the eti_b1_wm8731.c file to see what changes should be done. > >>> + start_event = channels == 1 >>> + ? 4 >>> + : 7; >> This would be much clearer if it were expanded into multiple statements. > > This was a little clearer in at91-ssc.c: > > start_event = channels == 1 > ? AT91_SSC_START_FALLING_RF > : AT91_SSC_START_EDGE_RF; > > Perhaps these constant definitions are no longer available it the latest > kernel. Are there updated definitions to use instead of magic numbers? My mistake, I will use your constant def. > >>> +#ifdef CONFIG_PM >>> +#define atmel_ssc_suspend NULL >>> +#define atmel_ssc_resume NULL >>> +#else >>> +#define atmel_ssc_suspend NULL >>> +#define atmel_ssc_resume NULL >>> +#endif >> These may as well be removed - if someone implements suspend/resume >> support they can add them then then. > > Is there a reason that suspend/resume was removed? It is really > important for embedded systems. I removed resume/suspend because I didn't have the time to write it... I wanted to add it in a next patch, but maybe I shoul do it now. Mark, concerning your other comments I am working on it. I will send another patch as soon as it is finished. Regards, Sedji _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
