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