Message ID | 20230807131256.254243-3-krzysztof.kozlowski@linaro.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:c44e:0:b0:3f2:4152:657d with SMTP id w14csp1487733vqr; Mon, 7 Aug 2023 07:24:37 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHjPcZOOYAZHk7bzDS4J5A/5bE9pJO0oPqfbKU23la7MPXU1hA8/1sHoWA63nNuWN2012Gz X-Received: by 2002:a17:906:56:b0:992:48b7:99e3 with SMTP id 22-20020a170906005600b0099248b799e3mr8563299ejg.63.1691418277439; Mon, 07 Aug 2023 07:24:37 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1691418277; cv=none; d=google.com; s=arc-20160816; b=o4nRcXDM1hZgh8T2C2B7YMXMTj/ePMEF2WcwkLndyDHdn4PkcGPHS/GUjRxTpdqqPo gdFn0SOJ1qXZN2z6WUfjkZyC6hNG1oDbI3AYu5Tns6UlCJqmNep7ZhTXuUyg4i1TxfTW xDuajdtKIP67biyK9XM+4cXRNfrrJcEX0zBz3boat8pGvgUa5/3/VU3wAoXAV6yOvUAO yVcpzdtZcp0HcJNq3biYakByCvlEZwFQekAEvLcozbwIlHTmOCL3iJOUbqsqci0C0YFr SQhEmbJ7vD4prpPaPgeJhyHhfzAgND+u3zyA3dXzh5C3kE2v0NcH0RZKuNB3u6CLCmmY 7H6Q== 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=DiXOG4vhE2jwuThX92Y67MrIjgQpotVRhoEdfPPU/2s=; fh=l5tDndJEyEuSXH8p3Wx1dfAyiqL2OB2SmLmAQu11EuQ=; b=BulmE74zwp/IapMyYTTJx+ZWGXlR7PjDNrn3zHXZUTBHqyEi1gRwIhTaJ8opKCLSTi qRZ3iobr+7roLBqThOCfKM7Ksl8KY2YQ10RndjBbKz7E5Jt3Xwe74oserYaXa6Skc7KG cA/hIANu3abSq0GDE6rOELS8Vahxoe5758JMO6cXmSjuUJ6yd70GIiS+BO6FXw5HR63o DTQBRjMw/sSwLjlzA7PZ4SPv4pXnqmiLrnmCEtlfwRz1epRDEFn4Lqr7p7HHH1TzmUtz O6qkPGGLvLgUcqqsd4caFWGeb4HbeKLe999Mdp6vwPQ9aRl8gSS0ergKh/0XAPt4P4JJ t6Cw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=UJfvJt5A; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id h16-20020a170906261000b0098e444cba76si4256202ejc.91.2023.08.07.07.24.13; Mon, 07 Aug 2023 07:24:37 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=UJfvJt5A; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232738AbjHGNNL (ORCPT <rfc822;aaronkmseo@gmail.com> + 99 others); Mon, 7 Aug 2023 09:13:11 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58862 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231593AbjHGNNI (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 7 Aug 2023 09:13:08 -0400 Received: from mail-wr1-x433.google.com (mail-wr1-x433.google.com [IPv6:2a00:1450:4864:20::433]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BDC6C10FE for <linux-kernel@vger.kernel.org>; Mon, 7 Aug 2023 06:13:05 -0700 (PDT) Received: by mail-wr1-x433.google.com with SMTP id ffacd0b85a97d-317f1c480eeso321347f8f.2 for <linux-kernel@vger.kernel.org>; Mon, 07 Aug 2023 06:13:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1691413984; x=1692018784; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=DiXOG4vhE2jwuThX92Y67MrIjgQpotVRhoEdfPPU/2s=; b=UJfvJt5AU+IWVBtlv78t+VTZeFaFm00LqJqWR6xVo2ENDxmCWPoNpDsLUjcv+1pevw w8N1ygAfMf1wUreBI0q5hYIpPFbnj0xcsFLV/hR/9CsUZiHntmTPIa0AsycPVzGqf6oM 72pp2uoiuXa7/jvaq3N9pgijqiVWgRBNDKUQyeiwk4aejKIR6N5Sq32SKfjwJyKrAhSJ +kSW12teQ1KrwbZZVGReE1tZ3pdcFx1MlbaMggxbFhSaOk/WIOXEO62sM1gmdGWEoXxx KFSzaBQe9gSvN+VFZioCg50Y1flr8gKMBc4GsR21Bj3kCjDA4OSrVmydpmxlMawYF7R9 acPw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1691413984; x=1692018784; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=DiXOG4vhE2jwuThX92Y67MrIjgQpotVRhoEdfPPU/2s=; b=IYr26rJxeb/qP9WlzXRwStchFw50rvQgFtipUQTVraKxckBnpWDDfTtkMzTKEl2f2i KbsswidUIILzLYNcl1kKr1KC2JhXmzBd45BjKUvwDlTIi+EZElQ4xJh1lLJgoiXIAjj4 nPix6c2v1HcxZPCusi7vN1qcflTcrCuqKxQ5KssARwtWp93Ox4C53VAyE3qHbOYxTo4O QuSKxlk+KPrazlQ/UaJ4Vkyoph08On63l+pu4Dy3znA2kun1lB+u/5vhIsz+FtOf+Bfe mvJ/65AA0dO3//hNkyJE6OjsgBbSjuW8yUKb1AJevwo8jk7+z1AwiPj+s8a108sbqp7L E1SQ== X-Gm-Message-State: AOJu0Yyb1eaSh6G47WDYR6j9MbqqhK2s2eCzJgWbRJxAVugWwTtH6kGP 8KBz0xOF1JFFU4rboEmBVfQ/6w== X-Received: by 2002:a05:6000:10a:b0:317:5b99:d3d7 with SMTP id o10-20020a056000010a00b003175b99d3d7mr5036961wrx.34.1691413984192; Mon, 07 Aug 2023 06:13:04 -0700 (PDT) Received: from krzk-bin.. ([178.197.222.113]) by smtp.gmail.com with ESMTPSA id j9-20020a5d4489000000b0031411b7087dsm10618428wrq.20.2023.08.07.06.13.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 07 Aug 2023 06:13:03 -0700 (PDT) From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> To: Sylwester Nawrocki <s.nawrocki@samsung.com>, Mauro Carvalho Chehab <mchehab@kernel.org>, Rob Herring <robh+dt@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley <conor+dt@kernel.org>, Alim Akhtar <alim.akhtar@samsung.com>, linux-media@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Subject: [PATCH v2 3/3] media: exynos4-is: fimc-is: replace duplicate pmu node with phandle Date: Mon, 7 Aug 2023 15:12:56 +0200 Message-Id: <20230807131256.254243-3-krzysztof.kozlowski@linaro.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230807131256.254243-1-krzysztof.kozlowski@linaro.org> References: <20230807131256.254243-1-krzysztof.kozlowski@linaro.org> 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 autolearn=unavailable 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-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1773580611858542614 X-GMAIL-MSGID: 1773580611858542614 |
Series |
[v2,1/3] media: dt-bindings: samsung,exynos4212-fimc-is: replace duplicate pmu node with phandle
|
|
Commit Message
Krzysztof Kozlowski
Aug. 7, 2023, 1:12 p.m. UTC
Devicetree for the FIMC IS camera included duplicated PMU node as its
child like:
soc@0 {
system-controller@10020000 { ... }; // Real PMU
camera@11800000 {
fimc-is@12000000 {
// FIMC IS camera node
pmu@10020000 {
reg = <0x10020000 0x3000>; // Fake PMU node
};
};
};
};
This is not a correct representation of the hardware. Mapping the PMU
(Power Management Unit) IO memory should be via syscon-like phandle
(samsung,pmu-syscon, already used for other drivers), not by duplicating
"pmu" Devicetree node inside the FIMC IS. Backward compatibility is
preserved.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Changes in v2:
1. Use IOMEM_ERR_PTR (Hans)
---
.../platform/samsung/exynos4-is/fimc-is.c | 33 ++++++++++++++-----
1 file changed, 24 insertions(+), 9 deletions(-)
Comments
Hi Krzysztof, [...] > +static void __iomem *fimc_is_get_pmu_regs(struct device *dev) > +{ > + struct device_node *node; > + void __iomem *regs; > + > + node = of_parse_phandle(dev->of_node, "samsung,pmu-syscon", 0); > + if (!node) { > + dev_warn(dev, "Finding PMU node via deprecated method, update your DTB\n"); > + node = of_get_child_by_name(dev->of_node, "pmu"); > + if (!node) > + return IOMEM_ERR_PTR(-ENODEV); in my opinion this should be: ... if (!node) return IOMEM_ERR_PTR(-ENODEV); dev_warn(dev, "Finding PMU node via deprecated method, update your DTB\n"); Because if you don't have both "samsung,pmu-syscon and "pmu" then the warning should not be printed and you need to return -ENODEV. ... and... "*please* update your DTB", the user might get upset and out of sheer spite, decides not to do it – just because! :) Andi > + }
On 08/08/2023 01:13, Andi Shyti wrote: > Hi Krzysztof, > > [...] > >> +static void __iomem *fimc_is_get_pmu_regs(struct device *dev) >> +{ >> + struct device_node *node; >> + void __iomem *regs; >> + >> + node = of_parse_phandle(dev->of_node, "samsung,pmu-syscon", 0); >> + if (!node) { >> + dev_warn(dev, "Finding PMU node via deprecated method, update your DTB\n"); >> + node = of_get_child_by_name(dev->of_node, "pmu"); >> + if (!node) >> + return IOMEM_ERR_PTR(-ENODEV); > > in my opinion this should be: > > ... > if (!node) > return IOMEM_ERR_PTR(-ENODEV); > > dev_warn(dev, "Finding PMU node via deprecated method, update your DTB\n"); > > Because if you don't have both "samsung,pmu-syscon and "pmu" then > the warning should not be printed and you need to return -ENODEV. Why not? Warning is correct - the driver is trying to find, thus continuous tense "Finding", PMU node via old method. > > ... and... "*please* update your DTB", the user might get upset > and out of sheer spite, decides not to do it – just because! :) The message is already long enough, why making it longer? Anyone who ships DTS outside of Linux kernel is doomed anyway... Best regards, Krzysztof
> >> +static void __iomem *fimc_is_get_pmu_regs(struct device *dev) > >> +{ > >> + struct device_node *node; > >> + void __iomem *regs; > >> + > >> + node = of_parse_phandle(dev->of_node, "samsung,pmu-syscon", 0); > >> + if (!node) { > >> + dev_warn(dev, "Finding PMU node via deprecated method, update your DTB\n"); > >> + node = of_get_child_by_name(dev->of_node, "pmu"); > >> + if (!node) > >> + return IOMEM_ERR_PTR(-ENODEV); > > > > in my opinion this should be: > > > > ... > > if (!node) > > return IOMEM_ERR_PTR(-ENODEV); > > > > dev_warn(dev, "Finding PMU node via deprecated method, update your DTB\n"); > > > > Because if you don't have both "samsung,pmu-syscon and "pmu" then > > the warning should not be printed and you need to return -ENODEV. > > Why not? Warning is correct - the driver is trying to find, thus > continuous tense "Finding", PMU node via old method. Alright, I'll go along with what you're suggesting, but I have to say, I find it misleading. From what I understand, you're requesting an update to the dtb because it's using deprecated methods. However, the reality might be that the node is not present in any method at all. Your statement would be accurate if you failed to find the previous method but then did end up finding it. Relying on the present continuous tense for clarity is a bold move, don't you think? :) Andi > > ... and... "*please* update your DTB", the user might get upset > > and out of sheer spite, decides not to do it – just because! :) > > The message is already long enough, why making it longer? Anyone who > ships DTS outside of Linux kernel is doomed anyway... > > Best regards, > Krzysztof >
On 08/08/2023 13:42, Andi Shyti wrote: >>>> +static void __iomem *fimc_is_get_pmu_regs(struct device *dev) >>>> +{ >>>> + struct device_node *node; >>>> + void __iomem *regs; >>>> + >>>> + node = of_parse_phandle(dev->of_node, "samsung,pmu-syscon", 0); >>>> + if (!node) { >>>> + dev_warn(dev, "Finding PMU node via deprecated method, update your DTB\n"); >>>> + node = of_get_child_by_name(dev->of_node, "pmu"); >>>> + if (!node) >>>> + return IOMEM_ERR_PTR(-ENODEV); >>> >>> in my opinion this should be: >>> >>> ... >>> if (!node) >>> return IOMEM_ERR_PTR(-ENODEV); >>> >>> dev_warn(dev, "Finding PMU node via deprecated method, update your DTB\n"); >>> >>> Because if you don't have both "samsung,pmu-syscon and "pmu" then >>> the warning should not be printed and you need to return -ENODEV. >> >> Why not? Warning is correct - the driver is trying to find, thus >> continuous tense "Finding", PMU node via old method. > > Alright, I'll go along with what you're suggesting, but I have to > say, I find it misleading. > > From what I understand, you're requesting an update to the dtb > because it's using deprecated methods. However, the reality might > be that the node is not present in any method at all. > > Your statement would be accurate if you failed to find the > previous method but then did end up finding it. > > Relying on the present continuous tense for clarity is a bold > move, don't you think? :) I just don't think it matters and is not worth resending. Best regards, Krzysztof
Hi Krzysztof, On 08/08/2023 01:13, Andi Shyti wrote: > Hi Krzysztof, > > [...] > >> +static void __iomem *fimc_is_get_pmu_regs(struct device *dev) >> +{ >> + struct device_node *node; >> + void __iomem *regs; >> + >> + node = of_parse_phandle(dev->of_node, "samsung,pmu-syscon", 0); >> + if (!node) { >> + dev_warn(dev, "Finding PMU node via deprecated method, update your DTB\n"); >> + node = of_get_child_by_name(dev->of_node, "pmu"); >> + if (!node) >> + return IOMEM_ERR_PTR(-ENODEV); > > in my opinion this should be: > > ... > if (!node) > return IOMEM_ERR_PTR(-ENODEV); > > dev_warn(dev, "Finding PMU node via deprecated method, update your DTB\n"); > > Because if you don't have both "samsung,pmu-syscon and "pmu" then > the warning should not be printed and you need to return -ENODEV. I agree with Andi for this part. The only time you want to see this message is if samsung,pmu-syscon is missing AND pmu is present. If both are missing, then just return ENODEV as it was before. > > ... and... "*please* update your DTB", the user might get upset > and out of sheer spite, decides not to do it – just because! :) I don't care about this bit. I guess it doesn't hurt to add 'please', but I accept it either way. Regards, Hans > > Andi > >> + }
diff --git a/drivers/media/platform/samsung/exynos4-is/fimc-is.c b/drivers/media/platform/samsung/exynos4-is/fimc-is.c index 530a148fe4d3..c995b1226ca3 100644 --- a/drivers/media/platform/samsung/exynos4-is/fimc-is.c +++ b/drivers/media/platform/samsung/exynos4-is/fimc-is.c @@ -767,12 +767,32 @@ static void fimc_is_debugfs_create(struct fimc_is *is) static int fimc_is_runtime_resume(struct device *dev); static int fimc_is_runtime_suspend(struct device *dev); +static void __iomem *fimc_is_get_pmu_regs(struct device *dev) +{ + struct device_node *node; + void __iomem *regs; + + node = of_parse_phandle(dev->of_node, "samsung,pmu-syscon", 0); + if (!node) { + dev_warn(dev, "Finding PMU node via deprecated method, update your DTB\n"); + node = of_get_child_by_name(dev->of_node, "pmu"); + if (!node) + return IOMEM_ERR_PTR(-ENODEV); + } + + regs = of_iomap(node, 0); + of_node_put(node); + if (!regs) + return IOMEM_ERR_PTR(-ENOMEM); + + return regs; +} + static int fimc_is_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; struct fimc_is *is; struct resource res; - struct device_node *node; int ret; is = devm_kzalloc(&pdev->dev, sizeof(*is), GFP_KERNEL); @@ -794,14 +814,9 @@ static int fimc_is_probe(struct platform_device *pdev) if (IS_ERR(is->regs)) return PTR_ERR(is->regs); - node = of_get_child_by_name(dev->of_node, "pmu"); - if (!node) - return -ENODEV; - - is->pmu_regs = of_iomap(node, 0); - of_node_put(node); - if (!is->pmu_regs) - return -ENOMEM; + is->pmu_regs = fimc_is_get_pmu_regs(dev); + if (IS_ERR(is->pmu_regs)) + return PTR_ERR(is->pmu_regs); is->irq = irq_of_parse_and_map(dev->of_node, 0); if (!is->irq) {