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.3 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 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 AE740C433E1 for ; Wed, 10 Jun 2020 10:36:53 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7B99520656 for ; Wed, 10 Jun 2020 10:36:53 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="Hm1CJi+k" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728320AbgFJKgv (ORCPT ); Wed, 10 Jun 2020 06:36:51 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55742 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726754AbgFJKgs (ORCPT ); Wed, 10 Jun 2020 06:36:48 -0400 Received: from mail-wm1-x344.google.com (mail-wm1-x344.google.com [IPv6:2a00:1450:4864:20::344]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 93513C03E96B for ; Wed, 10 Jun 2020 03:36:47 -0700 (PDT) Received: by mail-wm1-x344.google.com with SMTP id u13so1344603wml.1 for ; Wed, 10 Jun 2020 03:36:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=ct74x8QzTMiVHLIqtse4haJrkCu8RNQLYYWYvUjLwLQ=; b=Hm1CJi+kEcbRVQFtpTOZN8udhh8/3hHFsmWU8cKKb7GgkBWdzplDWxbxca7QPhiQOP yobx7OEX2ZL5XTjutp86nlWL5aIdX0DnOO9baFragUdKi/Pz2r9ThbZjpPT4Qpx4mNHR kq7wGhBzR7XaruQKWlIYVoirXRCylhmMSMntHMX2gZqavRGqOXoIgMVd1oVXlBGYbFin LvvUEDQy+f7Vidlgd8+19JawnPA6bz0cAKJoSvGmnRstw5iPZvs9eXnnkCMSvVtWxPk2 p1OJZfmhfCBnzlfMp31/+4epi1DOa/f0jxOAREiEw+mGS4ciRpsah7V5xwPBDpXnu7KB nxmg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=ct74x8QzTMiVHLIqtse4haJrkCu8RNQLYYWYvUjLwLQ=; b=VFJeoBgKSiJkXdUrf7k4slLeP5bv+Zo5RRejZCTHyWwPoi8qBKHRdh1aV38gPznh+k aqO6HDbZYdrW73pwYfKBGpxf7LXjq618fOrgiR/bHgOCB3cJYisINzgYv96kkFn0+Thx Xahxm0EG5DXvNq8Hpqx2vzih96va1j0bCdq3fXLJpZir36JNciRIe/nhDSaZq8V8HpZa +Dt2noNiLG4U2DrmcP6b9aoyxHeLEzRXGGQ3aSxnRFKtNJgjKsnjNEYbMwrlv+AT6KEc 272qY94hDuOf8oPNdOHCNUCysNyFAKhw1vmMnho2E5taZUrGq8/iMCFKjMHFtCciRWW5 KwOA== X-Gm-Message-State: AOAM531vLZdkITC8Hk4yGFtsNtIcb/3PTRdZ0frPynMDHLbvlMgxisis snuGCDQ9dp91uCz5wYYhzbsVucIcFP4= X-Google-Smtp-Source: ABdhPJz7r8bS7kNganx4qUD7P1fKicq2Q1nsivL8aGH8+2HEDFcVBi5XCoyamjLXo6DhqnROBkY57Q== X-Received: by 2002:a1c:2506:: with SMTP id l6mr2605714wml.34.1591785405839; Wed, 10 Jun 2020 03:36:45 -0700 (PDT) Received: from [192.168.86.34] (cpc89974-aztw32-2-0-cust43.18-1.cable.virginm.net. [86.30.250.44]) by smtp.googlemail.com with ESMTPSA id h27sm8781671wrb.18.2020.06.10.03.36.44 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Wed, 10 Jun 2020 03:36:45 -0700 (PDT) Subject: Re: [PATCH 4/5] soundwire: qcom: avoid dependency on CONFIG_SLIMBUS To: Jonathan Marek , alsa-devel@alsa-project.org Cc: Vinod Koul , Sanyog Kale , Pierre-Louis Bossart , Andy Gross , Bjorn Andersson , open list , "open list:ARM/QUALCOMM SUPPORT" References: <20200608204347.19685-1-jonathan@marek.ca> <20200608204347.19685-5-jonathan@marek.ca> From: Srinivas Kandagatla Message-ID: <29a216a7-a76f-a4de-b62d-3043e7e25c18@linaro.org> Date: Wed, 10 Jun 2020 11:36:44 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 09/06/2020 12:33, Jonathan Marek wrote: > On 6/9/20 5:52 AM, Srinivas Kandagatla wrote: >> >> >> On 08/06/2020 21:43, Jonathan Marek wrote: >>> The driver may be used without slimbus, so don't depend on slimbus. >>> >>> Signed-off-by: Jonathan Marek >>> --- >>>   drivers/soundwire/Kconfig | 1 - >>>   drivers/soundwire/qcom.c  | 5 +++++ >>>   2 files changed, 5 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/soundwire/Kconfig b/drivers/soundwire/Kconfig >>> index fa2b4ab92ed9..d121cf739090 100644 >>> --- a/drivers/soundwire/Kconfig >>> +++ b/drivers/soundwire/Kconfig >>> @@ -33,7 +33,6 @@ config SOUNDWIRE_INTEL >>>   config SOUNDWIRE_QCOM >>>       tristate "Qualcomm SoundWire Master driver" >>> -    depends on SLIMBUS >>>       depends on SND_SOC >> >> Why not move this to imply SLIMBUS this will give more flexibility! >> >> > > If you mean to change it to "select SLIMBUS", I'd prefer not to, because > this would increase code size unnecessarily in my kernel. imply is week select, which means that this driver can be built without SLIMBus selected. So removing reference to slimbus_bus is necessary in this case. On the other hand, SLIMBus is going to be used sm8250 for BT audio, so this would not be unnecessary. Also mostly these are build as modules, so not sure why kernel size will increase here! Am not 100% sure if SLIMBus will be kept on all SoCs, but keeping depends or imply in place would enforce or spell out some level of dependency on this. > >>>       help >>>         SoundWire Qualcomm Master driver. >>> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c >>> index 14334442615f..ac81c64768ea 100644 >>> --- a/drivers/soundwire/qcom.c >>> +++ b/drivers/soundwire/qcom.c >>> @@ -769,13 +769,18 @@ static int qcom_swrm_probe(struct >>> platform_device *pdev) >>>       if (!ctrl) >>>           return -ENOMEM; >>> +#ifdef CONFIG_SLIMBUS >>>       if (dev->parent->bus == &slimbus_bus) { >>> +#else >>> +    if (false) { >>> +#endif >> >> May be you can do bit more cleanup here, which could endup like: >> >> >> ctrl->regmap = dev_get_regmap(dev->parent, NULL); >> if (!ctrl->regmap) { >>      res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>      if (res) { >>          ctrl->mmio = devm_ioremap_resource(dev, res); >>          if (IS_ERR(ctrl->mmio)) { >>              dev_err(dev, "No valid mem resource found\n"); >>              return PTR_ERR(ctrl->mmio); >>          } >> >>          ctrl->reg_read = qcom_swrm_cpu_reg_read; >>          ctrl->reg_write = qcom_swrm_cpu_reg_write; >>      } else { >>          dev_err(dev, "No valid slim resource found\n"); >>          return -EINVAL; >>      } >> } else { >>      ctrl->reg_read = qcom_swrm_ahb_reg_read; >>      ctrl->reg_write = qcom_swrm_ahb_reg_write; >> } >> >> >> >> thanks, >> srini > > I don't think this is better, it feels more obfuscated, and I think its > possible we may end up with the mmio sdw having a parent with a regmap. > (it is not how I did things up in my upstream stack, but in downstream > the sdw nodes are inside the "macro" codec nodes) > > I understand the '#ifdef CONFIG_SLIMBUS'/'dev->parent->bus == > &slimbus_bus' is ugly, but at least its clear what's going on. Unless > you have a better suggestion? Other suggestion I had in my mind was to use compatible strings to get reg_read, reg_write callbacks + some flags like (if_type) populated. This can help looking up resources correctly. Thanks, srini > >>>           ctrl->reg_read = qcom_swrm_ahb_reg_read; >>>           ctrl->reg_write = qcom_swrm_ahb_reg_write; >>>           ctrl->regmap = dev_get_regmap(dev->parent, NULL); >>>           if (!ctrl->regmap) >>>               return -EINVAL; >>>       } else { >>> + >>>           res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>>           ctrl->reg_read = qcom_swrm_cpu_reg_read; >>>