Re: [bug report] ASoC: Intel: KMB: Enable TDM audio capture

[Dan Carpenter <dan.carpenter@oracle.com>]

On Tue, Aug 25, 2020 at 01:49:25PM +0000, Sit, Michael Wei Hong wrote:
> 
> 
> > On 25 Aug 2020, at 9:21 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > 
> > Hello Michael Sit Wei Hong,
> > 
> > The patch 9c3bab3c4f15: "ASoC: Intel: KMB: Enable TDM audio capture"
> > from Aug 11, 2020, leads to the following static checker warning:
> > 
> >    sound/soc/intel/keembay/kmb_platform.c:518 kmb_dai_hw_params()
> >    warn: potential ! vs ~ typo
> > 
> > sound/soc/intel/keembay/kmb_platform.c
> >   502          }
> >   503  
> >   504          config->chan_nr = params_channels(hw_params);
> >   505  
> >   506          switch (config->chan_nr) {
> >   507          case 8:
> >   508          case 4:
> >   509                  /*
> >   510                   * Platform is not capable of providing clocks for
> >   511                   * multi channel audio
> >   512                   */
> >   513                  if (kmb_i2s->master)
> >   514                          return -EINVAL;
> >   515  
> >   516                  write_val = ((config->chan_nr / 2) << TDM_CHANNEL_CONFIG_BIT) |
> >   517                                  (config->data_width << DATA_WIDTH_CONFIG_BIT) |
> >   518                                  !MASTER_MODE | TDM_OPERATION;
> >                                        ^^^^^^^^^^^^
> > MASTER_MODE is BIT(13).  It's unclear what this is supposed to be.  My
> > best guess is that the ! should just be deleted.
> 
> This ! is intentional because it is meant to be Slave mode. Would a better approach be to create another #define for slave mode?

In my opinion, it's better to just leave it out.  ORing with zero causes
a different static checker warning on my unreleased checks...  Is it
0 << 13?  I feel like ORing with zero just makes things more confusing.

regards,
dan carpenter