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=-1.0 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS, URIBL_BLOCKED autolearn=unavailable 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 ADEEDC43218 for ; Thu, 25 Apr 2019 19:44:15 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id C8AE9205ED for ; Thu, 25 Apr 2019 19:44:15 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="JvrQXxcy"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="sSaG+QmX" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C8AE9205ED Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date: Message-ID:From:References:To:Subject:Reply-To:Content-ID:Content-Description :Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=ySJAnyL077/Y67wAy19c/dwqlk8XwyhmHDcizYScIOc=; b=JvrQXxcyxGMrud 2nSdimpfwqIOtBcYAPFwp9A5yJHySndHW7jVJNwUUW+Z1T94dIlXYwJ7w2IgwCVnbmPv9R5F3g9Of STTXZenEJqKEGOg13l1wC5EPg39mIkbFOSvgbjlL5wxWpIJSG79P6IxQb12topzh1CIiIw89uhN8r Gp+xeaFoD3TRs8Ashacsu6XwGn44jb52NRFlZMoRthPpK6acxOKaAKU6p4rq6wH/Q0t3f43VHpp1H 4UrgDbMlwf0edSVqZz8PAjuVtXnbm9JvGRuyrHmfjWNDLo/YCtLe+WcuzBtxozsmVEkIjB1EeuiDf r8e785ioEuYP+9fjO7AQ==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1hJkHw-00042l-08; Thu, 25 Apr 2019 19:44:08 +0000 Received: from mail-wm1-x344.google.com ([2a00:1450:4864:20::344]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1hJkHt-00042L-4c for linux-arm-kernel@lists.infradead.org; Thu, 25 Apr 2019 19:44:06 +0000 Received: by mail-wm1-x344.google.com with SMTP id h11so867601wmb.5 for ; Thu, 25 Apr 2019 12:44:04 -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=Tt8mNhXexP5cg9uhSRxxyNlGYkAskhLpxbV2LkdxD6Y=; b=sSaG+QmXd+R5NUJEYgAPBcCi/9L6KrZYAMO6Wxet44YSiAA95Y2zEqQTRxEXLRfC90 C6IWDFNP+CgOuQmsdeqGcTN2K5GyXiB/pypQ1gReN5z72cfK++GZN1mnJpeSl/pG4ljQ 3bwxXykXAViZ20dfbg8o1pp4gMja6eXm2+WCyni1jTKbO61t8fKRxH+KYc2AhRiHw/+U SK3cvjdbDN6zT0F4ml5tyuKgVE7NS/Ia2Q04kSaeLzk3sKPpTe20TAyDZfolekf0VqOY 1E7NKDbayBxaKKEckHgd3Z7qvcDk1nVLjyuAnjS7CQqJD4vyEbP1KaKTya5BQVgmXffe +Mzg== 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=Tt8mNhXexP5cg9uhSRxxyNlGYkAskhLpxbV2LkdxD6Y=; b=tz06g8bWX+ZVQ0jvPvv4fGrBmDMiaW7BH6S0ppg5gRjc1rPhRmq/8cJfzik9u5GgMk M1gya57oihKFhVG9QhJIggE2pumW8mA8Oo4mN6fLev5HuxtalBzFv43c/X5VngglqlFw BlwZo0/t/esO8B+rPD8XQ241DL6psXD1VzK5eyzNgc3kooLuNSfW7o9PCHlnrxpqCA4f LRaFpx0KclefQzywugWyXiG/w0dOU9hSx7AcS+3tx+totmYWSr8gHfuFt/5Qn2k8yQAu kZ6TMKGpOC8x/5McTTbd+X5nKK37HodvQ0EudwGhlJoJoCimMPueVsC1Qid4wFKnfhfI t/8Q== X-Gm-Message-State: APjAAAUTTYHro1t5r/hQQwFvxWs3tpnhmhF2Vs0lJAPNec3IM6x/jZGJ Lqc63XHQe9eU+aZWv8TO5BzJIg== X-Google-Smtp-Source: APXvYqzcZVum/kmcNgir9ebf6JLC7hJy1u6Hu0SIMy3hz2CJfGPsx1QMCpNfu+dMUIpbY3Z+gnQ8rw== X-Received: by 2002:a1c:9691:: with SMTP id y139mr4467449wmd.64.1556221442727; Thu, 25 Apr 2019 12:44:02 -0700 (PDT) Received: from [192.168.1.2] (200.red-83-34-200.dynamicip.rima-tde.net. [83.34.200.200]) by smtp.gmail.com with ESMTPSA id g19sm21039391wmh.17.2019.04.25.12.44.00 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 25 Apr 2019 12:44:01 -0700 (PDT) Subject: Re: [PATCH 2/3] drivers: regulator: qcom: add PMS405 SPMI regulator To: Mark Brown References: <1548675904-18324-1-git-send-email-jorge.ramirez-ortiz@linaro.org> <1548675904-18324-3-git-send-email-jorge.ramirez-ortiz@linaro.org> <20190204090301.GC23441@sirena.org.uk> <95276ca0-6896-a595-867a-184a518fa31f@linaro.org> <20190425183736.GF23183@sirena.org.uk> From: Jorge Ramirez Message-ID: <022b3c6a-e356-3c5a-3c46-c6edcf4f8cd5@linaro.org> Date: Thu, 25 Apr 2019 21:44:00 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <20190425183736.GF23183@sirena.org.uk> Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190425_124405_191036_0608558B X-CRM114-Status: GOOD ( 22.02 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, vinod.koul@linaro.org, linux-kernel@vger.kernel.org, khasim.mohammed@linaro.org, lgirdwood@gmail.com, bjorn.andersson@linaro.org, robh+dt@kernel.org, linux-arm-msm@vger.kernel.org, niklas.cassel@linaro.org, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 4/25/19 20:37, Mark Brown wrote: > On Fri, Apr 19, 2019 at 07:29:48PM +0200, Jorge Ramirez wrote: >> On 2/4/19 10:03, Mark Brown wrote: > >>>> + /* we know we only have one range for this type */ >>>> + if (vreg->logical_type == SPMI_REGULATOR_LOGICAL_TYPE_HFS430) >>>> + return range; > >>> Rather than have special casing for the logical type in here it seems >>> better to just provide a specific op for this logical type, you could >>> always make _find_range() call into that one if you really want code >>> reuse here. You already have separate ops for this regulator type >>> anyway. > >> sorry I dont quite understand your point. > > If you need to skip the majority of the contents of the function for > some regulators just define a separate function for those regulators and > give them different ops structures rather than using the same ops > structure and handling it in the functions. sure this is 101 as a general rule: but this is not applicable to the situation that I described in my original note, so I dont think you read my points. > >> But also I am not sure I see the benefits with respect to the proposed >> change... > > The benefit is that the selection of which operations to use is done in > only one place (the selection of the ops structure) rather than in > multiple places (the selection of the ops structure and the contents of > the operations). all right, how do you propose that we handle spmi_regulator_select_voltage_same_range() then? the way I see it, if I follow your suggestion and since we are not allowed to extend spmi_regulator_find_range(), the only options are: 1) duplicate verbatim this whole function (spmi_regulator_select_voltage_same_range) with a minor change (this amount of code duplication in the kernel seems rather unnecessary to me) 2) modify the struct spmi_regulator definition with a new operation that calls a different implementation of find range (seems a massive overkill) because both seem wrong to me, can you confirm that you are ok with one of those two options? or if not give an alternative? But I still would like to understand why you think it is wrong extending spmi_regulator_find_range() to support HFS430. Are you saying that this function should not exist for this regulator? Sure the HFS430 doesnt use the register SPMI_COMMON_REG_VOLTAGE_RANGE and therefore doesnt need to use it to find its range....but that doesnt mean that the semantics of spmi_regulator_find_range are invalid. The way I understand your concern, you seem to be assuming that spmi_regulator_find_range means something like spmi_regulator_find_range_from_reg_voltage but that is not the case or if it is maybe it should be renamed..... _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel