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.6 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_ADSP_CUSTOM_MED,DKIM_SIGNED,DKIM_VALID,FSL_HELO_FAKE, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=no 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 DE35CC4338F for ; Wed, 11 Aug 2021 08:40:37 +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 A5F3260F41 for ; Wed, 11 Aug 2021 08:40:37 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org A5F3260F41 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=3mSP+1nwvFuf1ZwlKoQaXJ2Vz9/Pl8q4yKbvNiEgRfw=; b=XVIZBnThmpp/HG 9kqrGSiYIX98+IY1UYVb8whz8dBTgpdq+GsUnOpUqhLM5vC6nWz8fuWklIZ4wlujdYtaDMT9INsp9 HiZEDzQ9ZtAQvgxcJBwWkxiCoNmuslH/AJ7+7wrLvzZqfyCQCv0I3m38vP5oSwHfPhfOumw55A9H6 /KILpebeN4l8s4jkIVP9jnPPwKuMFtegIMtG65KM1GAKD5ZsR9VqgbNM4kIXi9TDI3wdoM/r+5Yqr jiyEE6QPGo4imikRy66SpW28FaL0YnWZLa0zQ2XTaTE2slUHPFSWBD5vDtsv8OUMOEI+PVwnhAVsR Zamxvlv086cpQC3uxb5A==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mDjkW-0062cf-6m; Wed, 11 Aug 2021 08:38:08 +0000 Received: from mail-wr1-x430.google.com ([2a00:1450:4864:20::430]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mDjkS-0062ba-FS for linux-arm-kernel@lists.infradead.org; Wed, 11 Aug 2021 08:38:05 +0000 Received: by mail-wr1-x430.google.com with SMTP id b13so1939604wrs.3 for ; Wed, 11 Aug 2021 01:38:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=8ctvExXHxoU7MDz0lLq/Tgi84JQatciTVAxD+Fn/jcQ=; b=rEq02tPeUoTDMJlCRt+ttFdA0Jr7daFhR+2ie5RBcOI3BzWNUuUWa9Gbkn4HGOW7WY 4dGznTdp6VXTPNE9W13rtYzeqx7tx7wQ393pENejnQSXz9eLj1RNBFRM1k3jq7xv7egq HC0wLX7Gfq1JdtcjNVDY4wLVGIFgnDphxa8SKvMeE6pZoYh5aVDsC/sruC06SYWsY3uM J7cw2lg+GbQUi7uuzGE1t6Q0KYY1VP89pD3oxKNUKWXB+QlwgXjHIwdMYwDYJtjhm4GA VV1DzVJB/z5IitVlWxedaVu1jjrQLyAUK9KtmHXlZnADSSCcDIKhD2TqwcqLbaym+Na8 XOdg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=8ctvExXHxoU7MDz0lLq/Tgi84JQatciTVAxD+Fn/jcQ=; b=lCt0XEmd5H+RfhFyf/QnGME2YKQUEtx/qf5RPyxI63AVDbziCWF92/ungQG36wLHPT YXoWgRG81EJEl5f0PNrGaHdCxVHwfrmefqeXBDPaTUquEyajg+kwcKd3PxzwLDBMt1Wy wLnfuh91wzvhfaB+9TOLX0v5AuUwUIjJ6r3YPQ5mLI+iH41agSoSj0HBcxhwxGXbzJxY RQnO4FPgx09HbKWb6kj7CDe/wg3z+P4la1cwN7wdJtAQx9//lIA9ngPadVXnjmRL8nzn xQQGkw1Ma/sMp2WmAyc+s5Pn48FDxL//vXyYZsT1mdS0245OCec6tdYoENp04FMq5dvV 7INg== X-Gm-Message-State: AOAM533VY8hKtPXvBLl98t7iIfigcVM5iuWN/2wjux82lsvSJiN+mHL7 I3dCTXdV95gS5RDP5Wb5pssBHw== X-Google-Smtp-Source: ABdhPJzU9YQR62yDNq9PrF7gkVKzwniyJFxIxg9cOJLgH9YZSN7yJXWq44plPsS4/Qn9sPmMW8M/QQ== X-Received: by 2002:a5d:490b:: with SMTP id x11mr35305440wrq.322.1628671082736; Wed, 11 Aug 2021 01:38:02 -0700 (PDT) Received: from google.com ([2a00:79e0:d:210:43fd:e634:73d9:e10e]) by smtp.gmail.com with ESMTPSA id e25sm13986334wra.90.2021.08.11.01.38.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 11 Aug 2021 01:38:02 -0700 (PDT) Date: Wed, 11 Aug 2021 09:37:56 +0100 From: Quentin Perret To: Viresh Kumar Cc: Rafael Wysocki , Vincent Donnefort , lukasz.luba@arm.com, Andy Gross , Bjorn Andersson , Cristian Marussi , Fabio Estevam , Kevin Hilman , Matthias Brugger , NXP Linux Team , Pengutronix Kernel Team , Sascha Hauer , Shawn Guo , Sudeep Holla , linux-pm@vger.kernel.org, Vincent Guittot , linux-arm-kernel@lists.infradead.org, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org, linux-omap@vger.kernel.org Subject: Re: [PATCH 0/8] cpufreq: Auto-register with energy model Message-ID: References: <20210811051859.ihjzhvrnuct2knvy@vireshk-i7> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210811051859.ihjzhvrnuct2knvy@vireshk-i7> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210811_013804_567469_F3F72014 X-CRM114-Status: GOOD ( 45.52 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wednesday 11 Aug 2021 at 10:48:59 (+0530), Viresh Kumar wrote: > On 10-08-21, 13:35, Quentin Perret wrote: > > On Tuesday 10 Aug 2021 at 13:06:47 (+0530), Viresh Kumar wrote: > > > Provide a cpufreq driver flag so drivers can ask the cpufreq core to register > > > with the EM core on their behalf. > > > > Hmm, that's not quite what this does. This asks the cpufreq core to > > use *PM_OPP* to register an EM, which I think is kinda wrong to do from > > there IMO. The decision to use PM_OPP or another mechanism to register > > an EM belongs to platform specific code (drivers), so it is odd for the > > PM_OPP registration to have its own cpufreq flag but not the other ways. > > > > As mentioned in another thread, the very reason to have PM_EM is to not > > depend on PM_OPP, so I'm worried about the direction of travel with this > > series TBH. > > I had to use the pm-opp version, since almost everyone was using that. > > On the other hand, there isn't a lot of OPP specific stuff in > dev_pm_opp_of_register_em(). It just uses dev_pm_opp_get_opp_count(), > that's all. This ended up in the OPP core, nothing else. Maybe we can > now move it back to the EM core and name it differently ? Well it also uses dev_pm_opp_find_freq_ceil() and dev_pm_opp_get_voltage(), so not sure how easy it will be to move, but if it is possible no objection from me. > > > This allows us to get rid of duplicated code > > > in the drivers and fix the unregistration part as well, which none of the > > > drivers have done until now. > > > > This series adds more code than it removes, > > Sadly yes :( > > > and the unregistration is > > not a fix as we don't ever remove the EM tables by design, so not sure > > either of these points are valid arguments. > > I think that design needs to be looked over again, it looks broken to > me everytime I land onto this code. I wonder why we don't unregister > stuff. > > Lets say, I am working on the cpufreq driver and I want to test that > on my ARM machine. Rebooting a simpler board to test stuff out is > easy, but if I am working on an ARM server which is running lots of > other userspace stuff as well, I won't want to reboot the machine just > to test a different versions of the driver. I will rather want to > build the driver as module and insert/remove it again and again. > > If the frequency table changes in between versions, this just breaks > as EM won't be updated again. > > This breaks one of the most basic rules of Linux Kernel. Inserting a > module should have exactly the same final behavior every single time. > This model doesn't guarantee it. It simply looks broken. Right but the EM is a description of the hardware, so it seemed fair to assume this wouldn't change across the lifetime of the OS, similar to the DT which we can't reload at run-time. Yes it can be a little odd if you load/unload your driver module, but note that you generally can't load two completely different drivers on a single system. You'll just load the same one again and the hardware hasn't changed in the meantime, so the previously loaded EM will still be correct. I hear your argument about cpufreq driver development, but the locking involved to allow 'just' that is pretty involved, and nobody has complained about this specific issue so far, so that didn't seem worth it. If we do have good reasons to change the EM at runtime, then yes I think we should do it, it just didn't seem like that was the case until now. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel