From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f49.google.com (mail-wm1-f49.google.com [209.85.128.49]) (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 85E7042A780 for ; Mon, 6 Jul 2026 12:36:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.49 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783341412; cv=none; b=i9vAhWCg9YZYbw72tuUztSiVAKmlHOhpkHGii4oHYHc294rk6rkXu44YYPfIJUJBL4gps9gUKtcYbXtrK/2BHYlD4qxu2fledLcwq8Vs7BLyeh476t64/1p2POZia3gJ/BFBaHg16rKo9r8H1CUwNrHpgKSovlTW5KTg2JNUQcc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783341412; c=relaxed/simple; bh=rra+HC6UW8ngg1M+w7ntsNH71CV+8l9s2Aq1rHgxNRs=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=pUx6s3ym2yIeAKPe/lF7p2EST7WprYLRum+UMk3NZaZwRqRXTjD9cF4WqwB51p1DPI8mFmW5gE/AUwk5VieDUIChAqS9R5BnoZVt6LtTlLQin4dbmwDyAEkV48qey6EJHFSND7+gJUXt+jmUqc0jR4OxJ4f8DcsjqEH+KEOz4Xs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=baylibre.com; spf=pass smtp.mailfrom=baylibre.com; dkim=pass (2048-bit key) header.d=baylibre.com header.i=@baylibre.com header.b=GZAqEqqt; arc=none smtp.client-ip=209.85.128.49 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=baylibre.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=baylibre.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=baylibre.com header.i=@baylibre.com header.b="GZAqEqqt" Received: by mail-wm1-f49.google.com with SMTP id 5b1f17b1804b1-493b77b150aso28215195e9.2 for ; Mon, 06 Jul 2026 05:36:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre.com; s=google; t=1783341404; x=1783946204; darn=lists.linux.dev; h=content-type:mime-version:message-id:date:user-agent:references :in-reply-to:subject:cc:to:from:from:to:cc:subject:date:message-id :reply-to:content-type; bh=3nillUBzOVKmupjRSjEGs6YGga/o/TlBfybdbWa9Q6w=; b=GZAqEqqtTRbNcXc4hMsTBcGHSVoaBAHe04MzeJ2Pzs4/XkTUwMC+oXU4SmU1h+W+sp YnT2TihDz01Wr4QUy5YvQ8cUfzByhltMHoF9YreWYsbj8BVzUs7aYOw44zefUU5MjISG xAaHWQnF6dzEXappOoBpvKjbhX3QME1gfjYaTuTSf6PljRYmAfFwiLIytQGWVR8RFyUX CNXQDjm6uyVuUUfTTZHB5mqVz5siQKlFLf6nubBUv6YwA5t2FiscLCKdFjztjkLwlpDK yJR8HCyo2bVjE/Dwm2PLlo8I3hhgwjCY9QpFHLGoug1ugoAMTdcQeD8vWCUsmGGziE0f SfkQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1783341404; x=1783946204; h=content-type:mime-version:message-id:date:user-agent:references :in-reply-to:subject:cc:to:from:x-gm-gg:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to:content-type; bh=3nillUBzOVKmupjRSjEGs6YGga/o/TlBfybdbWa9Q6w=; b=CQNUBKoI/l+u1HgHtB+gTA/2zn98RB0l11eS0gVn6fe32Ue/vqjO+6dBld2KKCKUl5 WQu1j6r81nn1u5v+/1WnhCbyO/AAOFQY49q15RLj85ACbZmtJ1OMhoyyLxaB0xHc7Q+x /NfO3hAvxQPiaQRG+YVkV9yMaO7UhXq87P7TvfaPT5vW9oBpJzGDe/O4qoJVEPo83j6n UbAo2oiWGSRCD0wCMrEofb27dJAtQb97GvxgoEXVffMixUptnwu73sFaqM0pWWFiv0lI iaR5tZGued6kcz9S+MXOChu49r/0YWAnGigbjwO0dhYHChbUTAFeieHU2HTmVGzNt38n FDoA== X-Forwarded-Encrypted: i=1; AHgh+RriBn4YAQVq5YblcBnUS77CpaENz1S3VLgrrtTjl3b6pMYTotyGNAXgTA4bdsnx9iwXEw2t8W9JV2QKZw==@lists.linux.dev X-Gm-Message-State: AOJu0YzJGQReB7pM8cGhfxjLRRbP2qg+YcO4Azhltj45SnPs2LeUGwEk YBJ8pWhWuRIM2bIqlykFpXt/rhxt2Kw4qRDxMGxc929qvGnZapXk2AyuCQH0Cc9iQzA= X-Gm-Gg: AfdE7clG0BudNOin6emLbZ1KjDAPjxWNKamHhLXCcNOSdh/lD3BcZ8nR7DVN+4XLEih 8wTjg2By9W+qVob/Ya2mOwWjRT8Lv7MuEqpP1C/eavIFAtDUgmz+pPXpYgigKlTbkHnK9sb9tms Oi7teOmT8oI9LBteFxwirZtREjwgtWYgsP7brPfjWly+drKZtlSLz7N7QuIkdgDuNGikud55xAN GtWAJnm1VvMmR/Zflex45NRM+F9/lwrtp72RlGcEr+LmF+HqpCCQQmUCwlzBBsG7lo2zKKMzl3u hqHog5J5jdZCaQTvUXfNW0KrSTLFG60FkSwfVQyHZDdOJRlhd/jrHy5RomLGdd0ExdFRNGWlryF 24GAaCQlKbdAWCYyA90lM2O+Pu1TF61Wiql18yDUo7gerAXWNhoxbUyWTsHjqk3LpBK82QnGQTm VlCDJV4usjarM= X-Received: by 2002:a05:600c:828e:b0:493:b6d5:72df with SMTP id 5b1f17b1804b1-493df090d47mr3392275e9.35.1783341404120; Mon, 06 Jul 2026 05:36:44 -0700 (PDT) Received: from localhost ([2a01:e0a:3c5:5fb1:1495:8c97:96f4:dff8]) by smtp.gmail.com with UTF8SMTPSA id 5b1f17b1804b1-493c637bc21sm660820945e9.7.2026.07.06.05.36.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 06 Jul 2026 05:36:43 -0700 (PDT) From: Jerome Brunet To: sashiko-bot@kernel.org Cc: sashiko-reviews@lists.linux.dev, conor+dt@kernel.org, robh@kernel.org, linux-rtc@vger.kernel.org, "Alexandre Belloni" , devicetree@vger.kernel.org, linux-sunxi@lists.linux.dev Subject: Re: [PATCH v4 6/9] clk: sunxi-ng: div: add read-only operation support In-Reply-To: <20260706100635.E5DC61F000E9@smtp.kernel.org> (sashiko-bot@kernel.org's message of "Mon, 06 Jul 2026 10:06:35 +0000") References: <20260706-a733-rtc-v4-0-f330728db3d3@baylibre.com> <20260706-a733-rtc-v4-6-f330728db3d3@baylibre.com> <20260706100635.E5DC61F000E9@smtp.kernel.org> User-Agent: mu4e 1.12.9; emacs 30.1 Date: Mon, 06 Jul 2026 14:36:41 +0200 Message-ID: <1jv7asguqu.fsf@starbuckisacylon.baylibre.com> Precedence: bulk X-Mailing-List: linux-sunxi@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain On lun. 06 juil. 2026 at 10:06, sashiko-bot@kernel.org wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [Medium] Inconsistent rate calculation between `determine_rate` and `recalc_rate` due to division order. > -- > > commit 39f872462c04ad724c3624b152234160de56a47e > Author: Jerome Brunet > > clk: sunxi-ng: div: add read-only operation support > > Add support for sunxi-ng read-only dividers. This will be > useful to the a733 oscillator detection logic. > >> diff --git a/drivers/clk/sunxi-ng/ccu_div.c b/drivers/clk/sunxi-ng/ccu_div.c >> index 62d680ccb524b..d1c8c7baa12dc 100644 >> --- a/drivers/clk/sunxi-ng/ccu_div.c >> +++ b/drivers/clk/sunxi-ng/ccu_div.c > [ ... ] >> @@ -84,6 +84,36 @@ static int ccu_div_determine_rate(struct clk_hw *hw, >> req, ccu_div_determine_rate_helper, cd); >> } >> >> +static int ccu_rodiv_determine_rate(struct clk_hw *hw, >> + struct clk_rate_request *req) >> +{ >> + struct ccu_div *cd = hw_to_ccu_div(hw); >> + unsigned long val; >> + u32 reg; >> + int ret; >> + >> + reg = readl(cd->common.base + cd->common.reg); >> + val = reg >> cd->div.shift; >> + val &= (1 << cd->div.width) - 1; >> + >> + req->rate = ccu_mux_helper_unapply_prediv(&cd->common, &cd->mux, -1, >> + req->rate); >> + >> + if (cd->common.features & CCU_FEATURE_FIXED_POSTDIV) >> + req->rate *= cd->fixed_post_div; >> + >> + ret = divider_ro_determine_rate(hw, req, cd->div.table, >> + cd->div.width, cd->div.flags, val); >> + >> + if (cd->common.features & CCU_FEATURE_FIXED_POSTDIV) >> + req->rate /= cd->fixed_post_div; >> + >> + req->rate = ccu_mux_helper_apply_prediv(&cd->common, &cd->mux, -1, >> + req->rate); > > [Severity: Medium] > Will this division order cause inconsistent rate calculations compared to > ccu_div_recalc_rate()? > > The divider_ro_determine_rate() helper applies DIV_ROUND_UP() to the unadjusted > parent rate, and then this function divides that result by fixed_post_div and > prediv. > > However, ccu_div_recalc_rate() applies prediv to the parent rate first, then > calculates the divider rate, and finally divides by fixed_post_div. > > Since the Common Clock Framework requires determine_rate and recalc_rate to be > mathematically consistent, could this discrepancy cause rate mismatches or > warnings for clocks using a pre-divider or fixed post-divider? I think the RO side is correct and RW one is not. The RO part correctly walk back the tree by un-applying the predivider so, whatever CCF does with the parent rate value will be correct. The RW part query the parent rate then applies the prediv. This will be thrown away when/if clk_divider_bestdiv() calls clk_hw_round_rate() on the parent. If that happens, the subsequent un-apply of the prediv would make the matter even worse. So (if I understand all this correctly) this case does not happen yet, otherwise we would have noticed, and it can be fixed separately. Side Note: I've not followed the sunxi history behind all those prediv/postdiv but I find them a bit confusing. It seems like inserting some fixed factor clocks in the clock trees would accomplish the same thing while simplifying mux/divider quite a lot by droping these quirks ? Side Note #2: I feel that while sashiko is pointing out valid things but we are slowly getting off topic and down the rabbit hole with it ... > >> + >> + return ret; >> +} >> + -- Jerome