Hi Andrew, On Fri, 2007-07-20 at 08:55 +0200, Andrew Victor wrote: > > Just an example from sound/soc/at91/at91-ssc.c > > > > rcmr = (( ssc_p->rcmr_period << 24) & AT91_SSC_PERIOD) > > | (( 1 << 16) & > > AT91_SSC_STTDLY) > > | (( AT91_SSC_START_FALLING_RF ) & AT91_SSC_START) > > | (( AT91_SSC_CK_RISING ) & AT91_SSC_CKI) > > | (( AT91_SSC_CKO_NONE ) & AT91_SSC_CKO) > > | (( AT91_SSC_CKS_DIV ) & AT91_SSC_CKS); > > Well, I didn't write the above, so it's more complex than it needs to > be. > > For bitfields where the user can input any value we would usually add > to > the header: > #define AT91_SSC_STTDLY_(x) ((x) << 16) > #define AT91_SSC_PERIOD_(x) ((x) << 24) > > Then it can simply be re-written as: > > rcmr = AT91_SSC_PERIOD(ssc_p->rcmr_period) > | AT91_SSC_STTDLY(1) > | AT91_SSC_START_FALLING_RF > | AT91_SSC_CK_RISING > | AT91_SSC_CKO_NONE > | AT91_SSC_CKS_DIV;
This is more sane and readable. > > Would with the header style for atmel-ssc be: > > > > rcmr = SSC_BF(RCMR_PERIOD, ssc_p->rcmr_period) > > | SSC_BF(RCMR_STTDLY, 1) > > | SSC_BF(RCMR_START, 4) > > | SSC_BF(RCMR_CKI, 1) > > | SSC_BF(RCMR_CKO, 0) > > | SSC_BF(RCMR_CKS, 0); > > > > I find the latter more readable and compact, the user also does not > need > > to know the offset of the different bit-fields. > > But the user does then constantly have to refer to the datasheet to > determine what CKI = 1 or CKS = 0 means. As I said two lines furter down, I think these defines are nice. Will adding bit-field descriptors make the SSC driver more usable by the AT91 people? I guess there is also the possibility to include the ARM headers for that architecture and use the definitions in the header file for AVR32. -- With kind regards, Hans-Christian Egtvedt, siv.ing. (M.Sc.) Applications Engineer - AVR32 System Solutions - Atmel Norway - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
