From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 177F7C00A89 for ; Fri, 30 Oct 2020 16:36:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A979D20727 for ; Fri, 30 Oct 2020 16:36:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727026AbgJ3Qgh (ORCPT ); Fri, 30 Oct 2020 12:36:37 -0400 Received: from mx2.suse.de ([195.135.220.15]:45496 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726348AbgJ3Qgh (ORCPT ); Fri, 30 Oct 2020 12:36:37 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 791B6ACF5; Fri, 30 Oct 2020 16:36:35 +0000 (UTC) Date: Fri, 30 Oct 2020 17:36:35 +0100 Message-ID: From: Takashi Iwai To: Pierre-Louis Bossart Cc: "Liao, Bard" , "alsa-devel@alsa-project.org" , "vinod.koul@linaro.org" , "Lin, Mengdong" , "linux-kernel@vger.kernel.org" , "hui.wang@canonical.com" , "vkoul@kernel.org" , "srinivas.kandagatla@linaro.org" , "ranjani.sridharan@linux.intel.com" , "jank@cadence.com" , Greg KH , "Kale, Sanyog R" , Bard Liao , "rander.wang@linux.intel.com" Subject: Re: [PATCH v3] soundwire: SDCA: add helper macro to access controls In-Reply-To: <880a22a2-49f6-fa1c-46ad-6388cbd26ee4@linux.intel.com> References: <20201029204955.8568-1-yung-chuan.liao@linux.intel.com> <20201030093651.GA2080962@kroah.com> <880a22a2-49f6-fa1c-46ad-6388cbd26ee4@linux.intel.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 30 Oct 2020 16:52:24 +0100, Pierre-Louis Bossart wrote: > > > > >>>> +#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. It's not necessarily mandatory to send the whole series, but if a relevant code is already available, mentioning a repo URL in the patch description (or in the comment below the delimiter) would be helpful, for example. > > 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. Hrm, OK, in this case the value is masked then shifted, so it's not trivial to deal with a macro. thanks, Takashi