Re: [PATCH v3] soundwire: SDCA: add helper macro to access controls

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





+#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




[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux