From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-lf1-f47.google.com (mail-lf1-f47.google.com [209.85.167.47]) (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 4CC56345CBC for ; Sun, 31 May 2026 10:18:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.167.47 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780222707; cv=none; b=PBimzZUXhsROBB0RETwtgCz9qaLGWMIZmXHgwmmgWH37hhtsdSSrYpjjxbl3WUiQoql7q49bNLW14HjE//DFZKOSiak+OnO3e/n/P0zzkpaPpapZ5WDV1sOaE1SRkI0/akMk4fZMvpn3uLMCg1HF+AFszdCId4UZh2ZVDecjiW8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780222707; c=relaxed/simple; bh=6pmOo4hPpgU1UDgVwFhbE+3Q+EAt9YB0MTkz9gFUMK0=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=P8wBsBnOc0dWBJsNoKUZTp3mJ4yQKNmfESmHvucwZMaxb5uWwgN1Y0lOuBESnzizXP7dxQrCq/MrRJnGfuC6xXb6gZM52/e6B7axdJkPd9vCzES+OGgrPfChMLlUDliAeeiJE73u/7uZkm3D+fWAxzXkJDOk26QsXsMfQ0eS63M= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=sa6AP2V4; arc=none smtp.client-ip=209.85.167.47 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="sa6AP2V4" Received: by mail-lf1-f47.google.com with SMTP id 2adb3069b0e04-5aa66893e9fso564803e87.1 for ; Sun, 31 May 2026 03:18:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1780222704; x=1780827504; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=T7IVg0Z/GtE0lxd89186+RKNusAM87gZCU/HngUTh/E=; b=sa6AP2V4mrTpJ4u2OnJeIe9YKxQqlpSy9DNV5NMisDYHuhfVtdCqqTX8ZcAp8Hne8n 4D1FHAcR0QAkLd/Hz+/v+uZBjOURXeV7p/Agz6DNSP95MZlWO46R/YNTw2KPRc4GYjEs tZGJ2lpB4fHtbzDPFF3GB5eArptcMCKitCGro/THnEnqbpmQ95kzUvAeWNF5R+ybCdmy g3p1T30auQ6X2mE0nFp734isuCXh7tACFWa6lBbNdhRDa6dkTL0ftSERd9SQzAM8gewl GfA4xTD1HzPAAmIzv1V9ihtEYZZtnV5YwmDAGdcKttBzE8VnBWr6O6TpoBuMbvpAfCNr AKEA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1780222704; x=1780827504; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=T7IVg0Z/GtE0lxd89186+RKNusAM87gZCU/HngUTh/E=; b=T5VHMmlBpdy+3yoZY4pMkbCmgd1oxvBwwbZCAmle/iv+6odKD0tJDeBjrUEzenCErz rha4mx8Rlft8ogtVzWBCtFKMGiND30lLtmcEDPmiIFzSPvdSfQXOUxTg7w9d7mVBWR9i 4sJZZL0EAdz783jYnaMq7/8gLtEE3hhgn9cviJX13syIdv/CymcTd/MJl7OwRWhD0pc1 O++XEG7ms4IP7jxXHUxHXGhH1OQ4G6jE0SFQcGSa8t+sk/qUmrrm0ZmgYxdxjMWUdts1 OE6P/PFJMRhG7vqcDADzN/QzoqYkoPUj22WCXYz3juPRSrOxHABu+mfJBSiw/6PcVHbP n3ng== X-Forwarded-Encrypted: i=1; AFNElJ/zdJy0yskdQ9g8thsQOg6JkFJB2yHqp0skyvH9j3+wTpKynHt1pEY271zCbDQ65Ojs6+rsWluBHxyb8UY=@vger.kernel.org X-Gm-Message-State: AOJu0YycTAXYeG7KdxTwsIUkvJ3EUw5EpvK9amSnLhyoPoKrppjZYB1x yavmOr7j+6H3B7cWskH+92oDeLsYPnoNBXk5RokhBsmwR3ER+2c1glXB X-Gm-Gg: Acq92OGSNkef7g3vYfGyHW9hUGjOZgfG9o1OWNML+/oI4RgUtVHyxyhGrUjlueG3/uV LeeiUEmIeP2sHx3Kg+kVHkdchKgnDxVA831p0DPRDIpHbrnnSa2mS2cOhnl9KMBzwRWhxJBE6Xh LcNHYMc0vPq+VxYHitcplB3eMF2EFHRJZfKrWOvwMq2eiEw3r+iRYLEDZXFTVuduDcjVxQi+nDE bPaDU9+Ja5XKbpuhqbWs/3/WPV0l8Wqv3aQhid8ezQqegXk86vr1ZJKw0TlJNg3dcWesc8o4nch EIHE9snGl81BlCjYgXxfIAKPVpQXRe6DfTxkw3bSHvJeLsrDhmEPr706EpZ/qd09V1Ax83ohaP1 p/oFmOAlWwHKqb++KQTXOUvIbo4Z8YrC5P/arNSdB8U15VnM/MzefR+j2HijuwXZp7L+j62oG+x lbTsJLb4T/PTaGlKtEMpZskBJcBvzbhi85x8K6UBKjEo6N/p4E0QYyJ0z4Q7iM5MX6/hSlDlE= X-Received: by 2002:a05:6512:308b:b0:5a8:80ce:ba55 with SMTP id 2adb3069b0e04-5aa607af794mr1763417e87.11.1780222704172; Sun, 31 May 2026 03:18:24 -0700 (PDT) Received: from gmail.com (83-233-6-197.cust.bredband2.com. [83.233.6.197]) by smtp.gmail.com with ESMTPSA id 38308e7fff4ca-39659e2966asm15125681fa.32.2026.05.31.03.18.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 31 May 2026 03:18:23 -0700 (PDT) Date: Sun, 31 May 2026 12:18:21 +0200 From: Marcus Folkesson To: Wolfram Sang Cc: Peter Rosin , Michael Hennerich , Bartosz Golaszewski , Andi Shyti , Andy Shevchenko , Bartosz Golaszewski , linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v9 1/5] i2c: core: add callback to change bus frequency Message-ID: References: <20260324-i2c-mux-v9-0-5292b0608243@gmail.com> <20260324-i2c-mux-v9-1-5292b0608243@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Hi Wolfram! On Tue, May 26, 2026 at 09:47:47PM +0200, Wolfram Sang wrote: > Hi Marcus, > > finally I found some time... thank you for your patience! No worries, I'm aware of your workload and appreciate you taking the time to review the patches. > > Some comments here, but I have high level questions first to be > discussed in patch 5. Maybe we should start there... > > > + int (*set_clk_freq)(struct i2c_adapter *adap, u32 clock_hz); /* Optional */ > > Shouldn't this rather go into 'struct i2c_algorithm'? Hrm, not sure. When looking into `struct i2c_algorithm`, it has no information about any other state. Also, the `struct i2c_algorithm` is often shared between multiple adapters, so if it had the information about the current clock frequency, it will be shared as well, which is not what we want. To me it feels more natural to have this callback in the adapter, but I'm open to change it if there are good reasons to do so. > > > +static inline int > > +__i2c_adapter_set_clk_freq(struct i2c_adapter *adapter, u32 clock_hz) > > +{ > > + if (adapter->set_clk_freq) > > + return adapter->set_clk_freq(adapter, clock_hz); > > So, Sashiko mentions[1] that setting 'adapter->clock_hz' below is not > executed. This is fine in my book because the requested new rate might > not be the actually used rate. However, I agree that it must be > documented that the callback is required to set 'clock_hz'. Or maybe > even better, the callback should return the newly set value and this > function then updates the variable? I will look into this, thanks! > > [1] https://sashiko.dev/#/patchset/20260324-i2c-mux-v9-0-5292b0608243%40gmail.com > > > + > > + /* > > + * If the adapter is a root adapter without .set_clk_freq() implemented, this feature is not > > + * supported. > > + */ > > + if (!i2c_parent_is_i2c_adapter(adapter)) > > + return -EOPNOTSUPP; > > + > > + /* > > + * Update the clock_hz for non-root adapters, even if .set_clk_freq() is not implemented, > > + * to allow the clock frequency to be propagated to root adapters that do support it. > > + */ > > + adapter->clock_hz = clock_hz; > > + return 0; > > So, Sashiko is IMO correct with asking what happens if setting the new > freq in the root adapter fails? There is no path to restore the old > values for the children then, or did I miss it? I will look into this as well. Spontaneously, I would say that if the root adapter fails to set the new frequency, the deselect chain will restore the old frequency for all children no matters what. > > Happy hacking, > > Wolfram > Best regards Marcus Folkesson