On 08.06.2023 13:43, Walker Chen wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 2023/6/8 18:15, Mark Brown wrote: >> On Thu, Jun 08, 2023 at 10:15:03AM +0800, Walker Chen wrote: >>> On 2023/6/7 19:44, Mark Brown wrote: >> >>>>> - (tdm->rx.wl << WL_BIT) | >>>>> - (tdm->rx.sscale << SSCALE_BIT) | >>>>> - (tdm->rx.sl << SL_BIT) | >>>>> - (tdm->rx.lrj << LRJ_BIT); >>>>> + datarx = (tdm->rxwl << 8) | >>>>> + (tdm->rxsscale << 4) | >>>>> + (tdm->rxsl << 2) | >>>>> + TDM_PCMRXCR_LEFT_J; >> >>>> I'm not sure this change to use numbers here is a win - the _BIT >>>> definitions look fine (I might've called them _SHIFT but whatever). >> >>> This is Claudiu's advice. Using the macro BIT() to replace these definition of *_BIT, >>> it will result in big changes in the code. >> >> I'm questioning doing a change at all. >> >>> Please refer to previous comments: >>> https://lore.kernel.org/all/143e2fa2-e85d-8036-4f74-ca250c026c1b@xxxxxxxxxxxxx/ >> >> I can't find the comments you're referring to in there. > > You should see the following comments in the link above: > >> + #define CLKPOL_BIT 5 >> + #define TRITXEN_BIT 4 >> + #define ELM_BIT 3 >> + #define SYNCM_BIT 2 >> + #define MS_BIT 1 > > Instead of these *_BIT defines as plain numbers you can defined them using > BIT() macro and use macros in place instead of > As mentioned in [1] I sent that by accident. Please ignore it and sorry for confusion. [1] https://lore.kernel.org/all/7a1a3ac3-10ec-9935-bca1-023cec6c0024@xxxxxxxxxxxxx/