Re: [PATCH 1/1][RESEND] Driver for the Atmel on-chip SSC on AT32AP and AT91.
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/
Re: [PATCH 1/1][RESEND] Driver for the Atmel on-chip SSC on AT32AP and AT91.
hi Hans-Christian, > 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; > 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. Regards, Andrew Victor - 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/
Re: [PATCH 1/1][RESEND] Driver for the Atmel on-chip SSC on AT32AP and AT91.
On Thu, 2007-07-19 at 10:29 +0200, Andrew Victor wrote: > hi Hans-Christian, > > > This driver is used in the ALSA sound driver for the AT73C213 external DAC > > on > > the ATSTK1000 development board for AVR32. This sound driver will be > > submitted > > soon. > > Seems to be more of an SCC unit allocation library, but OK. Correct, makes it easier for other modules to request the SSC peripheral and get access to the registers and irq. The AT73C213 sound driver was submitted to the alsa-dev list. > I've also forwarded a copy of the patch to Frank Mandarino > <[EMAIL PROTECTED]> for his comments. He maintains the current > AT91 SSC & ALSA support. > > > +/* SSC Receive Clock Mode Register */ > > +#define SSC_RCMR 0x0010 > > +#define SSC_RCMR_CKG_SIZE 2 > > +#define SSC_RCMR_CKG_OFFSET 6 > > +#define SSC_RCMR_CKI_SIZE 1 > > +#define SSC_RCMR_CKI_OFFSET 5 > > +#define SSC_RCMR_CKO_SIZE 3 > > I'm personally not a fan of the register & bit definition format the > AVR32 driver seem to use. > Can we not rather move the existing include/asm-arm/arch-at91/at91_ssc.h > to somewhere shared as atmel_ssc.h ? I disagree, by using the model above the user does not have to know the bit field offsets as they will need with the defines in include/asm-arm/arch-at91/at91_ssc.h. 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); 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. I agree that describing names for the different bit-fields is nice. Similar to the way the AVR32 standalone headers are built up. -- 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/
Re: [PATCH 1/1][RESEND] Driver for the Atmel on-chip SSC on AT32AP and AT91.
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/
Re: [PATCH 1/1][RESEND] Driver for the Atmel on-chip SSC on AT32AP and AT91.
On Thu, 2007-07-19 at 10:29 +0200, Andrew Victor wrote: hi Hans-Christian, This driver is used in the ALSA sound driver for the AT73C213 external DAC on the ATSTK1000 development board for AVR32. This sound driver will be submitted soon. Seems to be more of an SCC unit allocation library, but OK. Correct, makes it easier for other modules to request the SSC peripheral and get access to the registers and irq. The AT73C213 sound driver was submitted to the alsa-dev list. I've also forwarded a copy of the patch to Frank Mandarino [EMAIL PROTECTED] for his comments. He maintains the current AT91 SSC ALSA support. sound/soc/at91/ +/* SSC Receive Clock Mode Register */ +#define SSC_RCMR 0x0010 +#define SSC_RCMR_CKG_SIZE 2 +#define SSC_RCMR_CKG_OFFSET 6 +#define SSC_RCMR_CKI_SIZE 1 +#define SSC_RCMR_CKI_OFFSET 5 +#define SSC_RCMR_CKO_SIZE 3 I'm personally not a fan of the register bit definition format the AVR32 driver seem to use. Can we not rather move the existing include/asm-arm/arch-at91/at91_ssc.h to somewhere shared as atmel_ssc.h ? I disagree, by using the model above the user does not have to know the bit field offsets as they will need with the defines in include/asm-arm/arch-at91/at91_ssc.h. 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); 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. I agree that describing names for the different bit-fields is nice. Similar to the way the AVR32 standalone headers are built up. -- 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/
Re: [PATCH 1/1][RESEND] Driver for the Atmel on-chip SSC on AT32AP and AT91.
hi Hans-Christian, 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; 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. Regards, Andrew Victor - 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/
Re: [PATCH 1/1][RESEND] Driver for the Atmel on-chip SSC on AT32AP and AT91.
hi Hans-Christian, > This driver is used in the ALSA sound driver for the AT73C213 external DAC on > the ATSTK1000 development board for AVR32. This sound driver will be submitted > soon. Seems to be more of an SCC unit allocation library, but OK. I've also forwarded a copy of the patch to Frank Mandarino <[EMAIL PROTECTED]> for his comments. He maintains the current AT91 SSC & ALSA support. > +/* SSC Receive Clock Mode Register */ > +#define SSC_RCMR 0x0010 > +#define SSC_RCMR_CKG_SIZE 2 > +#define SSC_RCMR_CKG_OFFSET 6 > +#define SSC_RCMR_CKI_SIZE 1 > +#define SSC_RCMR_CKI_OFFSET 5 > +#define SSC_RCMR_CKO_SIZE 3 I'm personally not a fan of the register & bit definition format the AVR32 driver seem to use. Can we not rather move the existing include/asm-arm/arch-at91/at91_ssc.h to somewhere shared as atmel_ssc.h ? Regards, Andrew Victor - 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/
Re: [PATCH 1/1][RESEND] Driver for the Atmel on-chip SSC on AT32AP and AT91.
hi Hans-Christian, This driver is used in the ALSA sound driver for the AT73C213 external DAC on the ATSTK1000 development board for AVR32. This sound driver will be submitted soon. Seems to be more of an SCC unit allocation library, but OK. I've also forwarded a copy of the patch to Frank Mandarino [EMAIL PROTECTED] for his comments. He maintains the current AT91 SSC ALSA support. sound/soc/at91/ +/* SSC Receive Clock Mode Register */ +#define SSC_RCMR 0x0010 +#define SSC_RCMR_CKG_SIZE 2 +#define SSC_RCMR_CKG_OFFSET 6 +#define SSC_RCMR_CKI_SIZE 1 +#define SSC_RCMR_CKI_OFFSET 5 +#define SSC_RCMR_CKO_SIZE 3 I'm personally not a fan of the register bit definition format the AVR32 driver seem to use. Can we not rather move the existing include/asm-arm/arch-at91/at91_ssc.h to somewhere shared as atmel_ssc.h ? Regards, Andrew Victor - 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/
