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 Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 70B66C433EF for ; Fri, 26 Nov 2021 18:17:21 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 9A7108317A; Fri, 26 Nov 2021 19:17:18 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="UM5fio2t"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 118D8836C5; Fri, 26 Nov 2021 19:17:17 +0100 (CET) Received: from mail-qt1-x834.google.com (mail-qt1-x834.google.com [IPv6:2607:f8b0:4864:20::834]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 46F7C8317A for ; Fri, 26 Nov 2021 19:17:13 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=seanga2@gmail.com Received: by mail-qt1-x834.google.com with SMTP id o17so9750839qtk.1 for ; Fri, 26 Nov 2021 10:17:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=nn8xLnUWV7xEFHd0SddP5HphJhkXKkBdJHYIT5taJqQ=; b=UM5fio2tp9mgt97h64y3ODv3CkaO5vr4cYILyTXPA06paxVzQuTUyce39LHs5UYtTX TW9GvD8USpqnDYNtQimcCsS2+MQQSNbqsdEwvlzdbP26nh0pk+mMB3QSCbL5VYAPCrxY i9Vrigupd50V/1MMSBQfIReC23Yrv5fpkanN+I5O+Q2xGJTYlrmwVM+eINL9I7nVfGxX 2wOCc0MIptOXLZUiLMbJyNFBNkvamgSMxfAcgVW8BwiKKTm2PNERqtIWY6DOqjxc7xvN 6kxHYQJq3ybuNudI932h9swgQIx+aRKvushyOOUFoBaeDRjEkok3yAO2qW620DgoSaGz UAwA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=nn8xLnUWV7xEFHd0SddP5HphJhkXKkBdJHYIT5taJqQ=; b=VrGLCelmOBPDnjL9fXotr6Sz5kbxpBO17fi0O7IlS4zj1rjQnixOIAxonSpEWb9H1K DqQHMxH+gKtTyRaejmC8qDISlvYumGZaWFcLrO1/wVC5UFl1RKkyZW8FITmdyzUEbbbs 3oSciLQlu1X8xSwVP1xuUWDvHiXUQTf0Kyx2EdH2oqqL1TUBMr6tQvWWIi0bamJ/MVI0 3ku8XjJl0RfG+0tOOlteFkgZOL3jehjfewPNnxT6hMYTKI/nKFFvSky6W7B0EB5do6Yz /Isn3vvYHnc0k/3wBce2wxG58wyTcGsyxkAQV8QTSR8yB8Q51Dlt0HqmOQtankJ49+C1 itzw== X-Gm-Message-State: AOAM5330mAz5+ugcuQoHDQLHvAVMrgPgIi/l4As6HqsXaS17kVVDHu8O BNQUZhsZr1HpO9TTuaCrM2Kf/o4Jiwg= X-Google-Smtp-Source: ABdhPJzDSWaH4JchHPuuANuaIRx5PLNYbcx+oJZDVM//uDTqp5o4Mwm4/JimYJ9v/F/tX+iahEklUA== X-Received: by 2002:a05:622a:24c:: with SMTP id c12mr17246046qtx.483.1637950631823; Fri, 26 Nov 2021 10:17:11 -0800 (PST) Received: from [192.168.1.201] (pool-108-18-207-184.washdc.fios.verizon.net. [108.18.207.184]) by smtp.googlemail.com with ESMTPSA id y10sm3522504qkp.128.2021.11.26.10.17.10 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 26 Nov 2021 10:17:11 -0800 (PST) Subject: Re: [PATCH V2] clk: introduce u-boot,ignore-clk-defaults To: Tom Rini Cc: "Peng Fan (OSS)" , Rob Herring , "lukma@denx.de" , "sjg@chromium.org" , "u-boot@lists.denx.de" References: <20211029012801.15193-1-peng.fan@oss.nxp.com> <20211120125711.GO24579@bill-the-cat> <20211122132204.GV24579@bill-the-cat> <20211124141028.GS24579@bill-the-cat> From: Sean Anderson Message-ID: <5a180e8f-ad09-d907-9735-caac38992afc@gmail.com> Date: Fri, 26 Nov 2021 13:17:10 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: <20211124141028.GS24579@bill-the-cat> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.37 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean On 11/24/21 9:10 AM, Tom Rini wrote: > On Tue, Nov 23, 2021 at 09:16:14PM -0500, Sean Anderson wrote: >> On 11/22/21 10:02 PM, Peng Fan (OSS) wrote: >>>> Subject: Re: [PATCH V2] clk: introduce u-boot,ignore-clk-defaults >>>> >>>> On Mon, Nov 22, 2021 at 11:33:27AM +0800, Peng Fan (OSS) wrote: >>>>> + Rob >>>>> >>>>> On 2021/11/20 20:57, Tom Rini wrote: >>>>>> On Sat, Nov 20, 2021 at 12:10:54PM +0000, Peng Fan (OSS) wrote: >>>>>>>> Subject: [PATCH V2] clk: introduce u-boot,ignore-clk-defaults >>>>>>>> >>>>>>>> From: Peng Fan >>>>>>>> >>>>>>>> Current code has a force clk_set_defaults in multiple stages, >>>>>>>> U-Boot reuse the same device tree and Linux Kernel device tree, >>>>>>>> but we not register all the clks as Linux Kernel, so >>>>>>>> clk_set_defaults will fail and cause the clk provider registeration fail. >>>>>>>> >>>>>>>> So introduce a new property to ignore the default settings which >>>>>>>> could be used by any node that wanna ignore default settings. >>>>>>>> >>>>>>>> Reviewed-by: Simon Glass >>>>>>>> Signed-off-by: Peng Fan >>>>>>>> --- >>>>>>>> >>>>>>>> V2: >>>>>>>> Add R-b tag >>>>>>>> Tom, Simon >>>>>>>> After a thought, I think still put it as a u-boot thing. >>>> assigned-clock-x is >>>>>>>> actually Linux specific, however I could not add the new property >>>> to Linux, >>>>>>>> because we are supporting SystemReady-IR, we need the >>>>>>>> assigned-clock-x property >>>>>>>> in linux working and ignore it in U-Boot. >>>>>>> >>>>>>> Any more thoughts? >>>>>> >>>>>> Just my continued request that you treat this as generic and submit >>>>>> the binding upstream so it can be in the device tree for the platform. >>>>>> >>>>> >>>>> As Sean said, this is to serve cast that linux and U-Boot use the same >>>>> device tree, I mean U-Boot runtime export device tree to linux for >>>>> SR-IR (system-ready IR) booting. >>>>> >>>>> Linux needs assigned-clocks to some reason, but U-Boot not need that >>>>> because the driver not added the support or not a must to have that. >>>>> >>>>> Because assigned-clocks failure in U-Boot will cause probe fail now, >>>>> the device driver will report failure. >>>>> >>>>> You mean rename this to "ignore-clk-defaults" or keep >>>>> "u-boot,ignore-clk-defauls" or "firmware,ignore-clk-defaults" to linux >>>>> device tree binding? >>>>> >>>>> I could try to send to linux kernel with "firmware" as a prefix. >>>> >>>> What I mean is that first I'm not seeing the description of the property as >>>> being clear enough, either in commit message or the binding itself. >>>> That's why in my mind I keep seeing this as "we set the properties >>>> linux,assigned-clocks and u-boot,ignore-clk-defaults" and I don't know why >>>> we need both. Is there not something we can do based on seeing >>>> linux,assigned-clocks ? Showing something that makes use of the property >>>> you're wishing to add would be helpful. In fact, some specific dts snippets >>>> would be helpful to understand what's going on here. >>> >>> clk: clock-controller@30380000 { >>> compatible = "fsl,imx8mp-ccm"; >>> reg = <0x30380000 0x10000>; >>> #clock-cells = <1>; >>> clocks = <&osc_32k>, <&osc_24m>, <&clk_ext1>, <&clk_ext2>, >>> <&clk_ext3>, <&clk_ext4>; >>> clock-names = "osc_32k", "osc_24m", "clk_ext1", "clk_ext2", >>> "clk_ext3", "clk_ext4"; >>> assigned-clocks = <&clk IMX8MP_CLK_A53_SRC>, >>> <&clk IMX8MP_CLK_A53_CORE>, >>> <&clk IMX8MP_CLK_NOC>, >>> <&clk IMX8MP_CLK_NOC_IO>, >>> <&clk IMX8MP_CLK_GIC>, >>> <&clk IMX8MP_CLK_AUDIO_AHB>, >>> <&clk IMX8MP_CLK_AUDIO_AXI_SRC>, >>> <&clk IMX8MP_CLK_IPG_AUDIO_ROOT>, >>> <&clk IMX8MP_AUDIO_PLL1>, >>> <&clk IMX8MP_AUDIO_PLL2>; >>> assigned-clock-parents = <&clk IMX8MP_SYS_PLL1_800M>, >>> <&clk IMX8MP_ARM_PLL_OUT>, >>> <&clk IMX8MP_SYS_PLL2_1000M>, >>> <&clk IMX8MP_SYS_PLL1_800M>, >>> <&clk IMX8MP_SYS_PLL2_500M>, >>> <&clk IMX8MP_SYS_PLL1_800M>, >>> <&clk IMX8MP_SYS_PLL1_800M>; >>> assigned-clock-rates = <0>, <0>, >>> <1000000000>, >>> <800000000>, >>> <500000000>, >>> <400000000>, >>> <800000000>, >>> <400000000>, >>> <393216000>, >>> <361267200>; >>> u-boot,ignore-clk-defaults; >>> }; >>> >>> The above node is that I wanna have in U-Boot device tree. >>> This node is same as the one used in linux device tree except the new >>> property introduced here. >>> >>> In i.MX U-Boot, we not support the clks here in the clk driver, but linux needs the assigned-clocks property, so I could not delete it >>> because U-Boot runtime export the device tree to Linux. >>> >>> So add a new property here for U-Boot specific usage, if the property >>> is there, U-Boot ignore the assigned-clock settings for a node. >>> >>> I hope this is clear and please suggest what to do next. >> >> Hm. Well perhaps we can designate a specific return code to indicate >> that we have a valid clock ID but it is just unimplemented. So in your >> driver you would do >> >> ulong my_get_rate(struct clk *clk) >> { >> switch (clk->id) { >> case MY_CLK: >> ... >> default: >> if (clk->id < MY_CLK_MAX) >> return -ENOSYS; >> else >> return -EINVAL; >> } >> } >> >> and then the clock rate assignment would see -ENOSYS and go on its merry >> way. > > Or even just have the driver that matches on "fsl,imx8mp-ccm" have a > log_debug("Leaving clocks as-is, OS will handle them\n"); > and returning success? > assigned-clocks is handled by the uclass, so that seems a bit invasive for me. --Sean