lkml.org 
[lkml]   [2020]   [Oct]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v3] soundwire: SDCA: add helper macro to access controls
From
Date


>>>> +#define SDW_SDCA_CTL(fun, ent, ctl, ch) (BIT(30) |
>>> \
>>>> + (((fun) & 0x7) << 22) | \
>>>> + (((ent) & 0x40) << 15) | \
>>>> + (((ent) & 0x3f) << 7) | \
>>>> + (((ctl) & 0x30) << 15) | \
>>>> + (((ctl) & 0x0f) << 3) | \
>>>> + (((ch) & 0x38) << 12) | \
>>>> + ((ch) & 0x07))
>>>> +
>>>> +#define SDW_SDCA_MBQ_CTL(reg) ((reg) | BIT(13))
>>>> +#define SDW_SDCA_NEXT_CTL(reg) ((reg) | BIT(14))
>>>> +
>>>> #endif /* __SDW_REGISTERS_H */
>>>
>>>
>>> No users of these macros?
>>
>> SDW_SDCA_CTL is used in sdca codec drivers which are not upstream yet.
>> SDW_SDCA_MBQ_CTL will be used in a new regmap method.
>> SDW_SDCA_NEXT_CTL can be used in sdca codec drivers, too.
>
> Well, the point is that it's hard to review without seeing how the
> code of actual users are.

Agree, but our job is not made easy by the three-way dependency on
regmap, SoundWire before we can submit ASoC codec drivers (developed by
Realtek and tested by Intel).

If you prefer us to send all patches for SDCA codec support in one shot,
that would be fine with us.
> BTW, the bit definitions can be simplified with GENMASK().
> I personally don't think GENMASK() necessarily good, but it may fit
> better in a case like this.

we use this macro in switch cases, e.g. for regmap properties to define
read/volatile registers:

case SDW_SDCA_CTL(FUN_JACK_CODEC, RT711_SDCA_ENT_GE49,
RT711_SDCA_CTL_SELECTED_MODE, 0):
case SDW_SDCA_CTL(FUN_JACK_CODEC, RT711_SDCA_ENT_GE49,
RT711_SDCA_CTL_DETECTED_MODE, 0):
case SDW_SDCA_CTL(FUN_HID, RT711_SDCA_ENT_HID01,
RT711_SDCA_CTL_HIDTX_CURRENT_OWNER, 0) ...
SDW_SDCA_CTL(FUN_HID, RT711_SDCA_ENT_HID01,
RT711_SDCA_CTL_HIDTX_MESSAGE_LENGTH, 0):
case RT711_BUF_ADDR_HID1 ... RT711_BUF_ADDR_HID2:
return true;

https://github.com/thesofproject/linux/blob/70fe32e776dafb4b03581d62a4569f65c2f13ada/sound/soc/codecs/rt711-sdca-sdw.c#L35

and unfortunately all our attempts to use FIELD_PREP, FIELD_GET,
u32_encode, as suggested by Vinod, failed for this case due to
compilation issues (can't use these macros outside of a function scope).
The errors were shared with Vinod.

That's why we went back to the initial suggestion to deal with the
shifts/masks by hand. For now we don't have a better solution that works
in all cases were the macro is used.

Thanks
-Pierre

\
 
 \ /
  Last update: 2020-10-30 17:22    [W:0.122 / U:0.096 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site