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