[v3,09/20] soc: mediatek: mtk-svs: Move t-calibration-data retrieval to svs_probe()
Message ID | 20231121125044.78642-10-angelogioacchino.delregno@collabora.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:2b07:b0:403:3b70:6f57 with SMTP id io7csp595945vqb; Tue, 21 Nov 2023 04:54:49 -0800 (PST) X-Google-Smtp-Source: AGHT+IFpcqcaCxt9sJZt7e0vSaIqnQ9ZkATy7utagFKHOB+3Jl+J7cOjGwqWx8wenraXzVZz/xMP X-Received: by 2002:a17:90b:4f4e:b0:27c:f8bd:9a98 with SMTP id pj14-20020a17090b4f4e00b0027cf8bd9a98mr7063718pjb.40.1700571289015; Tue, 21 Nov 2023 04:54:49 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1700571289; cv=none; d=google.com; s=arc-20160816; b=rxMqRI1JSkfsWYFMBOaB7RoMJgWRizvNkbNXGQFM7wDP0yqV7RgHN1B9ddV38bB51W +t1sE7ForX4s+SRC4nwOTeYfXRcxddWQMYtN0m9RtSsLWo/k8sORRS12w2otHDqGRiNV Z6e+nKWwg4BNKc9c4Ss/7GbVwRKymtpAN5tf8ywDxtFRB1C4iiVuDmeVF3bmy5EmpVid n0zx9oYjsr2bOKpKkvDWRL5IHqVGx97woLJgjXIJmmal8ZAdxEV+t830Nos+Un/waDMc DknCcqNRLI1zLIvzxTKgsWuJ51atSR43tgKM/HspyXCDi45pmaNHIWhA/umhgD7wbZs0 Flzg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=/sTmpSUpUoBKaV3NeAFIU7m6PHTjrLFL8tG3Y3lCQEU=; fh=m5p6pcg3y47Sg5VKYWXj0f0nOzgFq4l5+GGYwGce+t4=; b=UDvny7uDrHMqMxxSQQ5mgAvmX8JjPg6y6XuxcGchX1HKz1DSVQEYe2LxVczWnSUzk+ xEPXd331pg86iS65Q09WdvlKLwooQFTOZEcCI0AANmq0QGIUF+V855IR5ZZSjFl03tak X/krSlxtJzYzhJ1F+gRKFdEbVI9cuP/JZUYajfbr0TUB1FUengCDPiY8O5UQyRFkcoge GRjF1naOFLAQnZzvXNZRjfgUtHIMmVb1C3lK3KEYdf55gbH3iIrkGUq/4D4hiWfBm3U5 OiiTQD4FJnF7xjaI/U744QZJ2DmEGG2bHuG+KlasAg+nIbwEJnpWZsivJ1xzksEmtmSQ SGQA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@collabora.com header.s=mail header.b=QXtid+iv; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=collabora.com Received: from snail.vger.email (snail.vger.email. [23.128.96.37]) by mx.google.com with ESMTPS id v12-20020a17090a898c00b00280386ec042si12608208pjn.149.2023.11.21.04.54.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 21 Nov 2023 04:54:48 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37; Authentication-Results: mx.google.com; dkim=pass header.i=@collabora.com header.s=mail header.b=QXtid+iv; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=collabora.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id 6B48E80A1875; Tue, 21 Nov 2023 04:51:39 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233878AbjKUMv0 (ORCPT <rfc822;ouuuleilei@gmail.com> + 99 others); Tue, 21 Nov 2023 07:51:26 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57792 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233758AbjKUMvF (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 21 Nov 2023 07:51:05 -0500 Received: from madras.collabora.co.uk (madras.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e5ab]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D0AAA1AA; Tue, 21 Nov 2023 04:51:01 -0800 (PST) Received: from IcarusMOD.eternityproject.eu (cola.collaboradmins.com [195.201.22.229]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: kholk11) by madras.collabora.co.uk (Postfix) with ESMTPSA id C8E4F6607314; Tue, 21 Nov 2023 12:50:59 +0000 (GMT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1700571060; bh=YAm2XwYefU2Wi2EmslwaPruVfwk8nKNQAa35BFgYSvI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=QXtid+ivp1dSih09MgnuxB4/ZRA4pjGsxDuGrhSKwwYcVK0UMLDH0eWsBzpUYBULB XkPzk17UGPJj9qII/QGy1AKPOVKl+k72O6cbbYaYsz6/S4b9lpuCJ632A6PYH659ZU LT5Lz5/KwXwbw326hHl0PO4uNsgCwNdxwoI1IiBmqgYAjyWUWN9VmMuYrW7Qv1kaRA hX2aEmUc5WgzHTf59V/Q8Ydty51DJ1ig8+cPlNb20L2uS21/kHxClGt+Hi+EreKmin OyvnPwbiaOb2uQs6pYcvDtn+X7oaV5EEn8v8GvJwv+i18NYdLuvx+H8PxOeNlkLqei xBuD6EZCNjuog== From: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> To: matthias.bgg@gmail.com Cc: krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, robh+dt@kernel.org, angelogioacchino.delregno@collabora.com, p.zabel@pengutronix.de, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, kernel@collabora.com, wenst@chromium.org Subject: [PATCH v3 09/20] soc: mediatek: mtk-svs: Move t-calibration-data retrieval to svs_probe() Date: Tue, 21 Nov 2023 13:50:33 +0100 Message-ID: <20231121125044.78642-10-angelogioacchino.delregno@collabora.com> X-Mailer: git-send-email 2.42.0 In-Reply-To: <20231121125044.78642-1-angelogioacchino.delregno@collabora.com> References: <20231121125044.78642-1-angelogioacchino.delregno@collabora.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Tue, 21 Nov 2023 04:51:39 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1783178239588244420 X-GMAIL-MSGID: 1783178239588244420 |
Series |
MediaTek SVS driver partial refactoring
|
|
Commit Message
AngeloGioacchino Del Regno
Nov. 21, 2023, 12:50 p.m. UTC
The t-calibration-data (SVS-Thermal calibration data) shall exist for
all SoCs or SVS won't work anyway: move it to the common svs_probe()
function and remove it from all of the per-SoC efuse_parsing() probe
callbacks.
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/soc/mediatek/mtk-svs.c | 32 ++++++--------------------------
1 file changed, 6 insertions(+), 26 deletions(-)
Comments
On 11/21/23 14:50, AngeloGioacchino Del Regno wrote: > The t-calibration-data (SVS-Thermal calibration data) shall exist for > all SoCs or SVS won't work anyway: move it to the common svs_probe() > function and remove it from all of the per-SoC efuse_parsing() probe > callbacks. > > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > --- > drivers/soc/mediatek/mtk-svs.c | 32 ++++++-------------------------- > 1 file changed, 6 insertions(+), 26 deletions(-) > > diff --git a/drivers/soc/mediatek/mtk-svs.c b/drivers/soc/mediatek/mtk-svs.c > index ab564d48092b..1042af2aee3f 100644 > --- a/drivers/soc/mediatek/mtk-svs.c > +++ b/drivers/soc/mediatek/mtk-svs.c > @@ -1884,11 +1884,6 @@ static bool svs_mt8195_efuse_parsing(struct svs_platform *svsp) > svsb->vmax += svsb->dvt_fixed; > } > > - ret = svs_get_efuse_data(svsp, "t-calibration-data", > - &svsp->tefuse, &svsp->tefuse_max); > - if (ret) > - return false; > - Hello Angelo, if you removed the code using `ret` in this patch, it makes sense to also remove the variable here instead of doing it in patch 18. It will avoid unused variable warnings for this patch. > for (i = 0; i < svsp->tefuse_max; i++) > if (svsp->tefuse[i] != 0) > break; > @@ -1949,11 +1944,6 @@ static bool svs_mt8192_efuse_parsing(struct svs_platform *svsp) > svsb->vmax += svsb->dvt_fixed; > } > > - ret = svs_get_efuse_data(svsp, "t-calibration-data", > - &svsp->tefuse, &svsp->tefuse_max); > - if (ret) > - return false; > - > for (i = 0; i < svsp->tefuse_max; i++) > if (svsp->tefuse[i] != 0) > break; > @@ -2009,11 +1999,6 @@ static bool svs_mt8188_efuse_parsing(struct svs_platform *svsp) > svsb->vmax += svsb->dvt_fixed; > } > > - ret = svs_get_efuse_data(svsp, "t-calibration-data", > - &svsp->tefuse, &svsp->tefuse_max); > - if (ret) > - return false; > - > for (i = 0; i < svsp->tefuse_max; i++) > if (svsp->tefuse[i] != 0) > break; > @@ -2097,11 +2082,6 @@ static bool svs_mt8186_efuse_parsing(struct svs_platform *svsp) > svsb->vmax += svsb->dvt_fixed; > } > > - ret = svs_get_efuse_data(svsp, "t-calibration-data", > - &svsp->tefuse, &svsp->tefuse_max); > - if (ret) > - return false; > - > golden_temp = (svsp->tefuse[0] >> 24) & GENMASK(7, 0); > if (!golden_temp) > golden_temp = 50; > @@ -2198,11 +2178,6 @@ static bool svs_mt8183_efuse_parsing(struct svs_platform *svsp) > } > } > > - ret = svs_get_efuse_data(svsp, "t-calibration-data", > - &svsp->tefuse, &svsp->tefuse_max); > - if (ret) > - return false; > - > /* Thermal efuse parsing */ > adc_ge_t = (svsp->tefuse[1] >> 22) & GENMASK(9, 0); > adc_oe_t = (svsp->tefuse[1] >> 12) & GENMASK(9, 0); > @@ -3040,8 +3015,13 @@ static int svs_probe(struct platform_device *pdev) > > ret = svs_get_efuse_data(svsp, "svs-calibration-data", > &svsp->efuse, &svsp->efuse_max); > + if (ret) > + return dev_err_probe(&pdev->dev, ret, "Cannot read SVS calibration\n"); With the previous code, if svs-calibration-data could not be read, the code would go to svs_probe_free_efuse. In your case, it returns directly. I believe that svs_get_efuse_data using nvmem_cell_read does not allocate the buffer for the efuse , hence no more need to free it ? The exit code is checking if it's ERR or NULL, but still, if the buffer was not allocated, it doesn't make sense to jump there indeed. In that case, you are also changing the behavior here , and your commit appears to do more than a simple move. > + > + ret = svs_get_efuse_data(svsp, "t-calibration-data", > + &svsp->tefuse, &svsp->tefuse_max); > if (ret) { > - ret = -EPERM; > + dev_err_probe(&pdev->dev, ret, "Cannot read SVS-Thermal calibration\n"); > goto svs_probe_free_efuse; again in this case the tefuse has not been allocated I assume. So previous code was a bit excessive in trying to free the efuse/tefuse ? Eugen > } >
Il 22/11/23 12:23, Eugen Hristev ha scritto: > On 11/21/23 14:50, AngeloGioacchino Del Regno wrote: >> The t-calibration-data (SVS-Thermal calibration data) shall exist for >> all SoCs or SVS won't work anyway: move it to the common svs_probe() >> function and remove it from all of the per-SoC efuse_parsing() probe >> callbacks. >> >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> >> --- >> drivers/soc/mediatek/mtk-svs.c | 32 ++++++-------------------------- >> 1 file changed, 6 insertions(+), 26 deletions(-) >> >> diff --git a/drivers/soc/mediatek/mtk-svs.c b/drivers/soc/mediatek/mtk-svs.c >> index ab564d48092b..1042af2aee3f 100644 >> --- a/drivers/soc/mediatek/mtk-svs.c >> +++ b/drivers/soc/mediatek/mtk-svs.c >> @@ -1884,11 +1884,6 @@ static bool svs_mt8195_efuse_parsing(struct svs_platform >> *svsp) >> svsb->vmax += svsb->dvt_fixed; >> } >> - ret = svs_get_efuse_data(svsp, "t-calibration-data", >> - &svsp->tefuse, &svsp->tefuse_max); >> - if (ret) >> - return false; >> - > > Hello Angelo, > > if you removed the code using `ret` in this patch, it makes sense to also remove > the variable here instead of doing it in patch 18. > It will avoid unused variable warnings for this patch. > > Yes, though the comment is not for this function, but rather for 8183. Anyway, that makes sense, but if it's the only change of this v3, it's something that I can fix while applying instead of sending another 20 patches round. Thanks. >> for (i = 0; i < svsp->tefuse_max; i++) >> if (svsp->tefuse[i] != 0) >> break; >> @@ -1949,11 +1944,6 @@ static bool svs_mt8192_efuse_parsing(struct svs_platform >> *svsp) >> svsb->vmax += svsb->dvt_fixed; >> } >> - ret = svs_get_efuse_data(svsp, "t-calibration-data", >> - &svsp->tefuse, &svsp->tefuse_max); >> - if (ret) >> - return false; >> - >> for (i = 0; i < svsp->tefuse_max; i++) >> if (svsp->tefuse[i] != 0) >> break; >> @@ -2009,11 +1999,6 @@ static bool svs_mt8188_efuse_parsing(struct svs_platform >> *svsp) >> svsb->vmax += svsb->dvt_fixed; >> } >> - ret = svs_get_efuse_data(svsp, "t-calibration-data", >> - &svsp->tefuse, &svsp->tefuse_max); >> - if (ret) >> - return false; >> - >> for (i = 0; i < svsp->tefuse_max; i++) >> if (svsp->tefuse[i] != 0) >> break; >> @@ -2097,11 +2082,6 @@ static bool svs_mt8186_efuse_parsing(struct svs_platform >> *svsp) >> svsb->vmax += svsb->dvt_fixed; >> } >> - ret = svs_get_efuse_data(svsp, "t-calibration-data", >> - &svsp->tefuse, &svsp->tefuse_max); >> - if (ret) >> - return false; >> - >> golden_temp = (svsp->tefuse[0] >> 24) & GENMASK(7, 0); >> if (!golden_temp) >> golden_temp = 50; >> @@ -2198,11 +2178,6 @@ static bool svs_mt8183_efuse_parsing(struct svs_platform >> *svsp) >> } >> } >> - ret = svs_get_efuse_data(svsp, "t-calibration-data", >> - &svsp->tefuse, &svsp->tefuse_max); >> - if (ret) >> - return false; >> - >> /* Thermal efuse parsing */ >> adc_ge_t = (svsp->tefuse[1] >> 22) & GENMASK(9, 0); >> adc_oe_t = (svsp->tefuse[1] >> 12) & GENMASK(9, 0); >> @@ -3040,8 +3015,13 @@ static int svs_probe(struct platform_device *pdev) >> ret = svs_get_efuse_data(svsp, "svs-calibration-data", >> &svsp->efuse, &svsp->efuse_max); >> + if (ret) >> + return dev_err_probe(&pdev->dev, ret, "Cannot read SVS calibration\n"); > > With the previous code, if svs-calibration-data could not be read, the code would > go to svs_probe_free_efuse. In your case, it returns directly. > I believe that svs_get_efuse_data using nvmem_cell_read does not allocate the > buffer for the efuse , hence no more need to free it ? The exit code is checking if > it's ERR or NULL, but still, if the buffer was not allocated, it doesn't make sense > to jump there indeed. > In that case, you are also changing the behavior here , and your commit appears to > do more than a simple move. > I'm not changing the behavior: the previous behavior was to fail and free the efuse variable if previously allocated, the current behavior is to fail and free the efuse variable if previously allocated, and the tefuse variable if previously allocated, which is a result of the actual move of the retrieval of the thermal fuse calibration data. I really don't see anything implicit here. >> + >> + ret = svs_get_efuse_data(svsp, "t-calibration-data", >> + &svsp->tefuse, &svsp->tefuse_max); >> if (ret) { >> - ret = -EPERM; >> + dev_err_probe(&pdev->dev, ret, "Cannot read SVS-Thermal calibration\n"); >> goto svs_probe_free_efuse; > > again in this case the tefuse has not been allocated I assume. > > So previous code was a bit excessive in trying to free the efuse/tefuse ? The previous code was performing an useless error check on something that was not supposed to be allocated *yet*. Yes, it was wrong before. Cheers, Angelo
On 11/22/23 14:41, AngeloGioacchino Del Regno wrote: > Il 22/11/23 12:23, Eugen Hristev ha scritto: >> On 11/21/23 14:50, AngeloGioacchino Del Regno wrote: >>> The t-calibration-data (SVS-Thermal calibration data) shall exist for >>> all SoCs or SVS won't work anyway: move it to the common svs_probe() >>> function and remove it from all of the per-SoC efuse_parsing() probe >>> callbacks. >>> >>> Signed-off-by: AngeloGioacchino Del Regno >>> <angelogioacchino.delregno@collabora.com> >>> --- >>> drivers/soc/mediatek/mtk-svs.c | 32 ++++++-------------------------- >>> 1 file changed, 6 insertions(+), 26 deletions(-) >>> >>> diff --git a/drivers/soc/mediatek/mtk-svs.c >>> b/drivers/soc/mediatek/mtk-svs.c >>> index ab564d48092b..1042af2aee3f 100644 >>> --- a/drivers/soc/mediatek/mtk-svs.c >>> +++ b/drivers/soc/mediatek/mtk-svs.c >>> @@ -1884,11 +1884,6 @@ static bool svs_mt8195_efuse_parsing(struct >>> svs_platform *svsp) >>> svsb->vmax += svsb->dvt_fixed; >>> } >>> - ret = svs_get_efuse_data(svsp, "t-calibration-data", >>> - &svsp->tefuse, &svsp->tefuse_max); >>> - if (ret) >>> - return false; >>> - >> >> Hello Angelo, >> >> if you removed the code using `ret` in this patch, it makes sense to >> also remove the variable here instead of doing it in patch 18. >> It will avoid unused variable warnings for this patch. >> >> > > Yes, though the comment is not for this function, but rather for 8183. > Anyway, that > makes sense, but if it's the only change of this v3, it's something that > I can fix > while applying instead of sending another 20 patches round. Thanks. > >>> for (i = 0; i < svsp->tefuse_max; i++) >>> if (svsp->tefuse[i] != 0) >>> break; >>> @@ -1949,11 +1944,6 @@ static bool svs_mt8192_efuse_parsing(struct >>> svs_platform *svsp) >>> svsb->vmax += svsb->dvt_fixed; >>> } >>> - ret = svs_get_efuse_data(svsp, "t-calibration-data", >>> - &svsp->tefuse, &svsp->tefuse_max); >>> - if (ret) >>> - return false; >>> - >>> for (i = 0; i < svsp->tefuse_max; i++) >>> if (svsp->tefuse[i] != 0) >>> break; >>> @@ -2009,11 +1999,6 @@ static bool svs_mt8188_efuse_parsing(struct >>> svs_platform *svsp) >>> svsb->vmax += svsb->dvt_fixed; >>> } >>> - ret = svs_get_efuse_data(svsp, "t-calibration-data", >>> - &svsp->tefuse, &svsp->tefuse_max); >>> - if (ret) >>> - return false; >>> - >>> for (i = 0; i < svsp->tefuse_max; i++) >>> if (svsp->tefuse[i] != 0) >>> break; >>> @@ -2097,11 +2082,6 @@ static bool svs_mt8186_efuse_parsing(struct >>> svs_platform *svsp) >>> svsb->vmax += svsb->dvt_fixed; >>> } >>> - ret = svs_get_efuse_data(svsp, "t-calibration-data", >>> - &svsp->tefuse, &svsp->tefuse_max); >>> - if (ret) >>> - return false; >>> - >>> golden_temp = (svsp->tefuse[0] >> 24) & GENMASK(7, 0); >>> if (!golden_temp) >>> golden_temp = 50; >>> @@ -2198,11 +2178,6 @@ static bool svs_mt8183_efuse_parsing(struct >>> svs_platform *svsp) >>> } >>> } >>> - ret = svs_get_efuse_data(svsp, "t-calibration-data", >>> - &svsp->tefuse, &svsp->tefuse_max); >>> - if (ret) >>> - return false; >>> - >>> /* Thermal efuse parsing */ >>> adc_ge_t = (svsp->tefuse[1] >> 22) & GENMASK(9, 0); >>> adc_oe_t = (svsp->tefuse[1] >> 12) & GENMASK(9, 0); >>> @@ -3040,8 +3015,13 @@ static int svs_probe(struct platform_device >>> *pdev) >>> ret = svs_get_efuse_data(svsp, "svs-calibration-data", >>> &svsp->efuse, &svsp->efuse_max); >>> + if (ret) >>> + return dev_err_probe(&pdev->dev, ret, "Cannot read SVS >>> calibration\n"); >> >> With the previous code, if svs-calibration-data could not be read, the >> code would go to svs_probe_free_efuse. In your case, it returns directly. >> I believe that svs_get_efuse_data using nvmem_cell_read does not >> allocate the buffer for the efuse , hence no more need to free it ? >> The exit code is checking if it's ERR or NULL, but still, if the >> buffer was not allocated, it doesn't make sense to jump there indeed. >> In that case, you are also changing the behavior here , and your >> commit appears to do more than a simple move. >> > > I'm not changing the behavior: the previous behavior was to fail and > free the efuse > variable if previously allocated, the current behavior is to fail and > free the > efuse variable if previously allocated, and the tefuse variable if > previously > allocated, which is a result of the actual move of the retrieval of the > thermal > fuse calibration data. > > I really don't see anything implicit here. > Previous behavior was ret = svs_get_efuse_data (efuse) if (ret) goto svs_probe_free_efuse Now, you have it as ret = svs_get_efuse_data (efuse) if (ret) return dev_err_probe... >>> + >>> + ret = svs_get_efuse_data(svsp, "t-calibration-data", >>> + &svsp->tefuse, &svsp->tefuse_max); >>> if (ret) { >>> - ret = -EPERM; >>> + dev_err_probe(&pdev->dev, ret, "Cannot read SVS-Thermal >>> calibration\n"); >>> goto svs_probe_free_efuse; >> >> again in this case the tefuse has not been allocated I assume. >> >> So previous code was a bit excessive in trying to free the efuse/tefuse ? > > The previous code was performing an useless error check on something > that was not > supposed to be allocated *yet*. Yes, it was wrong before. > > Cheers, > Angelo > _______________________________________________ > Kernel mailing list -- kernel@mailman.collabora.com > To unsubscribe send an email to kernel-leave@mailman.collabora.com
diff --git a/drivers/soc/mediatek/mtk-svs.c b/drivers/soc/mediatek/mtk-svs.c index ab564d48092b..1042af2aee3f 100644 --- a/drivers/soc/mediatek/mtk-svs.c +++ b/drivers/soc/mediatek/mtk-svs.c @@ -1884,11 +1884,6 @@ static bool svs_mt8195_efuse_parsing(struct svs_platform *svsp) svsb->vmax += svsb->dvt_fixed; } - ret = svs_get_efuse_data(svsp, "t-calibration-data", - &svsp->tefuse, &svsp->tefuse_max); - if (ret) - return false; - for (i = 0; i < svsp->tefuse_max; i++) if (svsp->tefuse[i] != 0) break; @@ -1949,11 +1944,6 @@ static bool svs_mt8192_efuse_parsing(struct svs_platform *svsp) svsb->vmax += svsb->dvt_fixed; } - ret = svs_get_efuse_data(svsp, "t-calibration-data", - &svsp->tefuse, &svsp->tefuse_max); - if (ret) - return false; - for (i = 0; i < svsp->tefuse_max; i++) if (svsp->tefuse[i] != 0) break; @@ -2009,11 +1999,6 @@ static bool svs_mt8188_efuse_parsing(struct svs_platform *svsp) svsb->vmax += svsb->dvt_fixed; } - ret = svs_get_efuse_data(svsp, "t-calibration-data", - &svsp->tefuse, &svsp->tefuse_max); - if (ret) - return false; - for (i = 0; i < svsp->tefuse_max; i++) if (svsp->tefuse[i] != 0) break; @@ -2097,11 +2082,6 @@ static bool svs_mt8186_efuse_parsing(struct svs_platform *svsp) svsb->vmax += svsb->dvt_fixed; } - ret = svs_get_efuse_data(svsp, "t-calibration-data", - &svsp->tefuse, &svsp->tefuse_max); - if (ret) - return false; - golden_temp = (svsp->tefuse[0] >> 24) & GENMASK(7, 0); if (!golden_temp) golden_temp = 50; @@ -2198,11 +2178,6 @@ static bool svs_mt8183_efuse_parsing(struct svs_platform *svsp) } } - ret = svs_get_efuse_data(svsp, "t-calibration-data", - &svsp->tefuse, &svsp->tefuse_max); - if (ret) - return false; - /* Thermal efuse parsing */ adc_ge_t = (svsp->tefuse[1] >> 22) & GENMASK(9, 0); adc_oe_t = (svsp->tefuse[1] >> 12) & GENMASK(9, 0); @@ -3040,8 +3015,13 @@ static int svs_probe(struct platform_device *pdev) ret = svs_get_efuse_data(svsp, "svs-calibration-data", &svsp->efuse, &svsp->efuse_max); + if (ret) + return dev_err_probe(&pdev->dev, ret, "Cannot read SVS calibration\n"); + + ret = svs_get_efuse_data(svsp, "t-calibration-data", + &svsp->tefuse, &svsp->tefuse_max); if (ret) { - ret = -EPERM; + dev_err_probe(&pdev->dev, ret, "Cannot read SVS-Thermal calibration\n"); goto svs_probe_free_efuse; }