From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-dl1-f45.google.com (mail-dl1-f45.google.com [74.125.82.45]) (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 5DB9B395D8E for ; Mon, 18 May 2026 20:59:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=74.125.82.45 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779137968; cv=none; b=kOJ+AumgIZ7AyCN9ojpIjfayp/Wr8T1Xi6yG1FpLWAAVxtJ9HHhGTSSnHqIZAU0tllpahzgEdAggAAToievwM6sz8g0uPDO4JNNxfYkEd9tc0e3vkmrDskDFyTLqQ2SYGiPqkS1swJttlUolGUNvEKSkjwAVPVoYNFFfEFhBe0k= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779137968; c=relaxed/simple; bh=nr+t1doD2CUBE3bFv8CLDzRSbdfYN9E47Pi8sfYRR1s=; h=Mime-Version:Content-Type:Date:Message-Id:Subject:From:To:Cc: References:In-Reply-To; b=RiGMvXjBPI6CHb0OJYzLOhDssGlpeCEszYGjen/ivladrJDAexJdALQix4qMdVMRcQkrWeEQJqW33rHlTT+iEa1I4AFxkTHfgIInye7WTYpo/eUpzfPx4tJSw3ttZBH+WMkeEClirrgi0I5Nx7d4PfaUmljSJ7dTlDn79hy3IiE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=nexthop.ai; spf=pass smtp.mailfrom=nexthop.ai; dkim=pass (2048-bit key) header.d=nexthop.ai header.i=@nexthop.ai header.b=dyg8CyiG; arc=none smtp.client-ip=74.125.82.45 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=nexthop.ai Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=nexthop.ai Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=nexthop.ai header.i=@nexthop.ai header.b="dyg8CyiG" Received: by mail-dl1-f45.google.com with SMTP id a92af1059eb24-134fe980658so3548497c88.1 for ; Mon, 18 May 2026 13:59:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nexthop.ai; s=google; t=1779137966; x=1779742766; darn=vger.kernel.org; h=in-reply-to:references:cc:to:from:subject:message-id:date :content-transfer-encoding:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=SC7LwadeZIJzGWSw3EwsmpwE/9tZMQSerDjTwBwBFWo=; b=dyg8CyiG/VlbC8sQRcH/JjZXEm5u1XUoiIqxbkpLrehrqPHg0iK1hzgn4NTt7kX4d0 WPDVmlMYp3hvN3fu7FrGeEE4gaaNPs989pSDCbe1a6VQahMmTdvZphWer5eMvTwiggy8 HSEhZglqvOVqge5oDlApG6QDaGWGmUNUcziTLdV+9KVjwbL8LYqYET0bbAq+nX4u7P++ PIkJvWO52iROzOAwXTbFTS4l2EUEPo+Ixenz5VHLE7+3tTZsDwNUqsFMoewFiYiVDAsj jFygJp7bmMPsByOq52/0QTK4jj3YXcr2ltcgkCzBOeAu/encTZD/Wsl2rxHz21kf27oM Tkiw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1779137966; x=1779742766; h=in-reply-to:references:cc:to:from:subject:message-id:date :content-transfer-encoding:mime-version:x-gm-gg:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=SC7LwadeZIJzGWSw3EwsmpwE/9tZMQSerDjTwBwBFWo=; b=QfQFncHa4O/Jjuf9z/Xahi/XDDoqz1ojNi8NV+QEF3jaJNqgM78mNWgzIl13kXSsPq 5W+T6skeGlpNIM58SRh5MbO5epwWEEDKw+ZytVY5/lhUNYczaJ+WfTmFMeLyubh+aacD 0O9Hsl67QUXPukfJeu+zZhblO243VifJ/4mPs/WT8Ad4ss0IGJkyupfKZ2TdbP9j4sC6 b6WT+o/ySFc/9WSg6PVV+nC1D4CY76IhK7WvYQvGMnVivKxr35l1bURhT3XsfxxTPl12 66hrAKddwmhXTpMeTXN4XKHraxzrusaop3FyNrDvor3/znFXNTatoQzFTMWj5xsmAuQi y9eQ== X-Forwarded-Encrypted: i=1; AFNElJ/KJQC2YOXzF3e5loTKkN/UjE8kNWzz2uE3+a/XKxjUkQ37DeGM7cG8Qa/PBlwK8u31McMtyK7pvsZTt00=@vger.kernel.org X-Gm-Message-State: AOJu0YwXaF7LGPbtu2gCT75mDEahlIPiFbv+6czd4JLQDtRBJnqM4vkK gs8sV3s1bXp11kvmpXND47+mW/62QBmrPcP6bpwuIIrlsvDDk9b04204i8bl3zFtPMI= X-Gm-Gg: Acq92OHkfBriLB4dHlnf4+N8EYWhs79qYLhN2N86IZ9or4tbR8eefEXeuEDdRRbOVIu OJNwU5D0eV6NiCEiODNR/ajOGzF0lEGLfKqMG92UiMlh1PbM0Q+TYHmTJe9yHQLYG5fjGALPyUj BpN+kzR3+JnA4av1wcWrjJ9Isnr7zXOej3fe616fN9QuHWIC7vLtmWB/4+unPHH0XCphkRHua2o TvJP1bluReBS4asiGwC4nU6RtFzvd19QBFgIiQErolo2pGfptDm0ODt/Lj53422XRaepS49h6IE 46MYuaHMOh793yUpopmzVdYrwQeQP+en6wEiujEB1HylOJCqjAvYjUJW7bcC+q9ZwX+yRDZ1wTv AJHbYPcQIPZwoWg/RG9mOIX/FWrNHVUdzVPvBbmYPBdF/B+EBsDazFmkq8cVTCm8C56tFfz0WK7 Nk7nJI0Ia8Z6ly7LSkBZ1V1Vk= X-Received: by 2002:a05:7300:dc8e:b0:2f2:5c68:5074 with SMTP id 5a478bee46e88-3039818af8fmr7531363eec.13.1779137966208; Mon, 18 May 2026 13:59:26 -0700 (PDT) Received: from localhost ([50.145.100.174]) by smtp.gmail.com with ESMTPSA id 5a478bee46e88-30296dcb6f6sm18597898eec.17.2026.05.18.13.59.25 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 18 May 2026 13:59:25 -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: Mon, 18 May 2026 13:59:25 -0700 Message-Id: Subject: Re: [PATCH v3] i2c: mux: reg: use device property accessors From: "Abdurrahman Hussain" To: "Abdurrahman Hussain" , "Peter Rosin" Cc: , X-Mailer: aerc 0.21.0 References: <20260513-i2c-mux-reg-v3-1-0fa7242605bc@nexthop.ai> In-Reply-To: On Mon May 18, 2026 at 1:15 PM PDT, Abdurrahman Hussain wrote: > On Mon May 18, 2026 at 6:59 AM PDT, Peter Rosin wrote: >> >> Hi! >> >> After reading v2/v3, I finally got why you removed if (!mux->data.reg). >> I.e., since you removed the io-remapping from i2c_mux_reg_probe_fw() >> that part can now be unconditional in i2c_mux_reg_probe(). But, >> keeping the if just to accomodate potential out-of-tree code that uses >> the platform data hooking is not needed so v1 feels better than v2/v3. >> Sorry for being dense. However, see below. >> > > Agreed -- v4 will drop the guard and make > devm_platform_get_and_ioremap_resource() unconditional. > > Follow-up question while we're pruning the platdata path: with the > guard gone, the .reg and .reg_size fields in > struct i2c_mux_reg_platform_data are functionally dead (any value a > platdata user supplied through them is overwritten by > devm_platform_get_and_ioremap_resource()). Should I drop those two > fields from the struct in the same patch, or leave them as silent > no-ops in the public header? Happy either way. > Scratch that, sorry -- right after sending I noticed drivers/platform/mellanox/mlx-platform.c is an in-tree platdata user. It registers i2c-mux-reg platform_devices via platform_device_register_resndata(..., res=3DNULL, num_res=3D0, ...) and supplies a pre-set .reg (a (void __iomem *) cast of an LPC register address) and .reg_size=3D1 through the platdata struct (mlxplat_default_mux_data[] and friends around line 474). Without the if (!mux->data.reg) guard, devm_platform_get_and_ioremap_resource() would run unconditionally, fail because the pdev carries no memory resource, and break mlx-platform's mux probe. v4 will therefore keep the guard, and the .reg / .reg_size fields stay in the public platdata struct. The other two changes stand: - i2c_get_adapter_by_fwnode() -> i2c_find_adapter_by_fwnode() + put_device() in probe_fw(), matching the OF original. - In probe(), move the if (!mux->data.reg) ioremap block -- together with the reg_size validation that depends on it -- above the i2c_get_adapter() call. fwnode users get back the master-side ordering of "ioremap before parent-adapter get"; platdata users see the size check run a bit earlier, but mux->data.reg_size is set from platdata by then, so it's order-neutral for them. Cheers, Abdurrahman >>> - adapter =3D of_find_i2c_adapter_by_node(adapter_np); >>> - of_node_put(adapter_np); >>> + >>> + adapter =3D i2c_get_adapter_by_fwnode(fwnode); >>> + fwnode_handle_put(fwnode); >> >> Why did you not change to i2c_find_adapter_by_fwnode()? It would be >> closer to the original, but maybe that doesn't work for ACPI for some >> reason? >> > > It works for ACPI -- both _find and _get bottom out in > i2c_find_adapter_by_fwnode(). My reason for picking the _get > variant was simply that it does try_module_get(adapter->owner) on > top. But i2c_get_adapter() further down in probe() already does > the same try_module_get() and -EPROBE_DEFERs on failure, so the > extra acquire-then-immediately-release in probe_fw() isn't earning > us anything. v4 will switch to i2c_find_adapter_by_fwnode() + > put_device(), matching the OF original more closely. > >>> - /* map address from "reg" if exists */ >>> - if (of_address_to_resource(np, 0, &res) =3D=3D 0) { >>> - mux->data.reg_size =3D resource_size(&res); >>> - mux->data.reg =3D devm_ioremap_resource(&pdev->dev, &res); >>> - if (IS_ERR(mux->data.reg)) >>> - return PTR_ERR(mux->data.reg); >>> - } >>> - >> >> By not doing the io-remapping here, the ordering is now subtly altered. >> Previously we had io-remapping before the call to i2c_get_adapter(). >> With this change we instead have the call to i2c_get_adapter() before >> the io-remapping. >> >> Since it is apparently difficult the get this patch tested for the >> existing use cases, we should avoid as many of these innocent looking >> but subtle changes as possible. >> >> Cheers, >> Peter >> > > Will fix that in v4 by reordering probe() so the > devm_platform_get_and_ioremap_resource() call comes before > i2c_get_adapter(), matching the master sequence. No need to push > the ioremap back into probe_fw() -- the helper signature stays > cleaner if probe() owns the call. > > Thanks for the careful re-read. > > Cheers, > Abdurrahman