From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f46.google.com (mail-wr1-f46.google.com [209.85.221.46]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id ABE6C41D65F for ; Thu, 16 Jul 2026 12:57:51 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.46 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784206673; cv=none; b=qEOTmmUoW3zL6XGqyjkQGnQlRUYtwv83/7Y1BakGhq+Zr7ETI5p4Cgk5cEcrfHH+8c5mfg0Oh6CIyXYyRXEm5fJDfRFojj7X5QiqmjAgG0bCZK7Mp7s42uvhUVDu8DseNxSb8q7xorWilLaQIRTJy1Y43lBibX1Ua5jpQkp1/b0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784206673; c=relaxed/simple; bh=ms7AIU8iFFaoa0Ma9a1omLId+z2jTN6MXG5dawWMtgY=; h=Mime-Version:Content-Type:Date:Message-Id:Subject:From:To:Cc: References:In-Reply-To; b=a1D7uPShiEnokml4LaVogeZewcWxwh2BwLEPGDDz3Fo/0QNARRX4eJQL461EM8WbjVie026jpuTMQrS2e0OP3vkVr8robftznINndWZyezWIM9uqxmbS9zFgBmvGmgewxmVLfEK5pZ0udjfbPZeWrvehwbb7lPzzYTS5bJVP6wM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org; spf=pass smtp.mailfrom=linaro.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b=jAcAcAuq; arc=none smtp.client-ip=209.85.221.46 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linaro.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="jAcAcAuq" Received: by mail-wr1-f46.google.com with SMTP id ffacd0b85a97d-47ddf7b09e5so2893029f8f.1 for ; Thu, 16 Jul 2026 05:57:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1784206669; x=1784811469; darn=vger.kernel.org; h=in-reply-to:references:cc:to:from:subject:message-id:date :content-type:content-transfer-encoding:mime-version:from:to:cc :subject:date:message-id:reply-to:content-type; bh=0LYFgy4yfYQhPuAiPz9CB3wYdVuPpzUCUHrbqHln8gc=; b=jAcAcAuqmFZDp2BO+tdEPoUbfhKkQyvzes1jODCJv0PIynlz/YQ97EoaO2J01jCbVf PJUOS+8Pn52MdTXPpM5GOVUoj/q+YgHrTocxhbnTWVgsug3sckL8DpG3XvEOKQ5XizvZ L2rFYLjAHzk6wXeT9bsIIfpH4+Akwp7AkF5pqfxa04H2Mzmhcd25Z/PWov/J687fU+y6 6e51yVQrLpLnYXNJxXa8TCa/32lyJej/j/0lq0dvlySvyFWEmAbCTLGVmPof6MGK4hlV nJy2v9KXSk7uy2dBY+LubmuF82XoM8L487JuVfEhlRWLkcOF7QM8dPbJG/5cf6Un5ua0 vaLw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1784206669; x=1784811469; h=in-reply-to:references:cc:to:from:subject:message-id:date :content-type:content-transfer-encoding:mime-version:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=0LYFgy4yfYQhPuAiPz9CB3wYdVuPpzUCUHrbqHln8gc=; b=JXg3y2z7fy6xT7E2COJDQRYBWDxHkOSuHQ9CKjepqrteteOnYvONubfIpvavRnVaUE AFvT31b9JY6etfCksOn7q3C3YZSOZ6OocL2DRpe6Vh80X80/1nH6+V9fh//PUJAAlc1o undSM5Fv09g3XMA7z6CrnhsjGlMAGdjiLJsqpw6TiCx1T6zOKYdiQe+YrrZvxOCsr+la qnI2SIw8HbpLBBdVJQ8/A1Iokt4tgYSBOWkCsUO/vafT8XFI7vhuXpJnIQKmVVR4Dr6v Em0C0iCxR4Y+KAz8Oxo5GiAY/ap3n3SxvouEPu/8yy9ji7Sk65EJaN3jCADOTMjGz7Xx Bv2g== X-Forwarded-Encrypted: i=1; AHgh+Rp+JeWsjM5zR4g+UQb/BMsUjI3BQgX/74q+KUss3lRw7ePxx27r1+JB8SVEerG8yo5A9T4GOWT7JdlGHRY=@vger.kernel.org X-Gm-Message-State: AOJu0YwZOaH1wBQ3fwk1CzGVU2En8uRuE8j0C0aIQ0o8noSryv0pTRI7 ddcRTvN3fB4oK2ranZkjVH+Fuor+UUJJ+6Hv7EFUpwXC+R07Rdjxe9EJHH3fG6WhlQQ= X-Gm-Gg: AfdE7cljZasieRKxk4mxA3qMc16z/EspmnwYmgA8xkCR86xQ9uoHe3bPyIFBRJaPJU0 x/7qNnA7KITROMkL/0QhjNwjyJjjw7p5so8my1uSu+Dn4gbC3qNiXTk8Y7Ud5POhbCgW297/rX1 xxDoy4fZQO8qNvm5tU47EDHWAkO+4ztOQami4O1SFYs/0ter17u1TgxIOPMduDDrEkxKSN9CTwd /PdXcNaPRt3vpjiyIK+Ky4hRbSkC45GhJ3kuVr4I4HK/1JdQcJsFV4zbXJ4XQo7bca7XRVX+K8F iOFaosuxLuKLBD+r/HlXEGraLSPknlQk9xSpgU6tslAl9At51INmsQvHzoX1+68NCQXN17vmfZb 0JQusfg3g/IigUy43DfgmROtFNea0Ie7xQESJSUFU7b9MhnDWWFL8ImMtUGzh5RFgK17bZ9dTM3 Ek1190O+/ac1h+0I9B94/wNJhv4vfKbFy0V5WtowtxV2FCTb1Elq123LnT1n//tTnFsd5x X-Received: by 2002:a05:6000:1867:b0:473:1ccc:15cc with SMTP id ffacd0b85a97d-47f2da80da8mr24985592f8f.0.1784206669165; Thu, 16 Jul 2026 05:57:49 -0700 (PDT) Received: from localhost ([2a00:2381:fd67:101:5e58:c548:d5e9:96bf]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-47f464b7e22sm23722294f8f.25.2026.07.16.05.57.48 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 16 Jul 2026 05:57:48 -0700 (PDT) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Thu, 16 Jul 2026 13:57:47 +0100 Message-Id: Subject: Re: [PATCH 3/3] soc: samsung: exynos-pmu: fix error paths in cpuhotplug/idle states setup From: "Alexey Klimov" To: "Peter Griffin" Cc: "Krzysztof Kozlowski" , "Alim Akhtar" , "Sam Protsenko" , , , , , "Sashiko" X-Mailer: aerc 0.20.0 References: <20260605-exynos-pmu-cpuhp-idle-fixes-v1-0-0cd05c81a82d@linaro.org> <20260605-exynos-pmu-cpuhp-idle-fixes-v1-3-0cd05c81a82d@linaro.org> In-Reply-To: Hi Peter, On Mon Jun 22, 2026 at 9:43 PM BST, Peter Griffin wrote: > Hi Alexey, > > On Mon, 22 Jun 2026 at 19:57, Alexey Klimov wr= ote: > [..] >> >> > >> >> > I originally made the failure non-fatal because the system still bo= ots >> >> > without the notifiers registered (and all other Arm64 Exynos SoCs >> >> > upstream don't register notifiers and AFAICT have broken cpu hotplu= g >> >> > and cpu idle). >> >> > >> >> > In hindsight, that seems like a mistake. I think your patch to full= y >> >> > unwind everything in case of failure makes more sense. See small >> >> > comment below about destroy_cpuhp_and_cpuidle() >> >> >> >> Wait, setup_cpuhp_and_cpuidle() should be non-fatal and shouldn't >> >> return any errors? >> > >> > I suggest you re-read my above comment above ^^ >> >> Could you please clarify what specifically addresses my question about >> notifiers? > > Sure, I was referring to this part of my previous reply: > >> >> > In hindsight, that seems like a mistake. I think your patch to full= y >> >> > unwind everything in case of failure makes more sense. > > [..] >> >> If c2 idles are used during reboot/shutdown then they fail or what? > > This followed similar logic to the Samsung downstream kernel drivers. > I have no extra information about it beyond the downstream kernel > source. It seemed reasonable though that CPU's will be hotplugged > during suspend and reboot so you may wish to ignore these. > > The proper solution of course is a fully PSCI compliant firmware, > which doesn't require these side channel hints. It seems there is some misunderstanding, where we are not getting to the point. I think I still do not see any comment about failed notifiers and question about c2 idle states is kinda misdirected. I also don't see any answers that clarify this from previous emails: >Why do we need to have notifiers (say cpu_pm_register_notifier()) >registered if, for instance, cpuhp_setup_state() fails? Let's jump to the details then to move it forward somewhere. If one of the cpuhp_setup_state() fails and in the current sources (without this patch) we don't check if it succeeds or not we may not have properly working hotplug notifiers =3D> {set,clear}_bit(cpu, pmu_context->in_cpuhp) may not be updated correctly. The if-check logic in cpuidle callback: if (test_bit(cpu, pmu_context->in_cpuhp)) { raw_spin_unlock(&pmu_context->cpupm_lock); return NOTIFY_BAD; } relies on that and may not work reliably. As a result c2 idle states may stop working or offlined CPU may be put into c2 idle state. Going further, if register_reboot_notifier() fails and we don't check its return value then /* Ignore CPU_PM_ENTER event in reboot or suspend sequence. */ if (pmu_context->sys_insuspend || pmu_context->sys_inreboot) { raw_spin_unlock(&pmu_context->cpupm_lock); return NOTIFY_OK; } a CPU can be put or at least attempted to be put into c2 idle state. That's why there was a question: >> If c2 idles are used during reboot/shutdown then they fail or what? If it is fine to use c2 idle states during power off or reboot, then why the code blocks them during power off/reboot? >> I am not saying that patch is correct and some rework is needed but I do= n't >> get why we should completely ignore errors from hotplug states registrat= ion >> and should not check registration of notifiers. At least warning should = be >> shown to user that pm functionality might be unreliable. > > As mentioned above, and in my previous reply, I think your proposed > patch is a good idea. You mentioned, so far as I understood your replies, that _only_ destroy_cpuhp_and_cpuidle() is useful and setup_cpuhp_and_cpuidle() should be non-fatal. I still think there are some needed error paths for setup_cpuhp_and_cpuidle() as described above. Best regards, Alexey.