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 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 1EB68C7EE31 for ; Fri, 27 Jun 2025 11:17:04 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id CE86C10E9CA; Fri, 27 Jun 2025 11:17:03 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="HwUpA+xb"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.9]) by gabe.freedesktop.org (Postfix) with ESMTPS id AFD8610E9CA for ; Fri, 27 Jun 2025 11:17:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1751023023; x=1782559023; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=XE+SNt4kCGXPHHVnVBp5uHmdhW6OwyVs1LNsF1QzHQU=; b=HwUpA+xb0nv5HIyiuI/OaFoGiI8wbsp7H4wbm5OEnb9QW5daj0JTV72k fAULeZXNzjcRSGgEEGDxCYLfcvzAxjSR44x6+RmIMEOaCyUlxC7yapvQJ 2Jvwy7kcZnflQjCCTGPboO9I5uLk5j8jTvLoSBrOSNpNpp/qomqoA8ZTB pYd50K9lIXqRBTNIhKdYmO8xDnfdpRrVmTdR+a528yagtCpK6Punw/oy2 QX9whLgbnUm2TGkL1SA85lASJ7Z12ksWpAl49Oq8/E2ln5pEGaZB3A6LA 7R3tumFbumsM5NOzF7cQgiZnv4IpMZlrn8Nafr8fPHubvSn4VeqO4u2GT A==; X-CSE-ConnectionGUID: THVGJmczQCS25COghZyahg== X-CSE-MsgGUID: VtE6WwgoQqOVWy8Lu/0mig== X-IronPort-AV: E=McAfee;i="6800,10657,11476"; a="75881761" X-IronPort-AV: E=Sophos;i="6.16,270,1744095600"; d="scan'208";a="75881761" Received: from fmviesa001.fm.intel.com ([10.60.135.141]) by orvoesa101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Jun 2025 04:17:02 -0700 X-CSE-ConnectionGUID: dIAnwJDEQ7WGLe82EQUPNQ== X-CSE-MsgGUID: Wx7byjf+QAyzYlBWvWrgKg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.16,270,1744095600"; d="scan'208";a="183679231" Received: from smile.fi.intel.com ([10.237.72.52]) by fmviesa001.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Jun 2025 04:16:58 -0700 Received: from andy by smile.fi.intel.com with local (Exim 4.98.2) (envelope-from ) id 1uV74h-0000000ASge-0kdF; Fri, 27 Jun 2025 14:16:55 +0300 Date: Fri, 27 Jun 2025 14:16:54 +0300 From: Andy Shevchenko To: Heikki Krogerus Cc: Lucas De Marchi , Thomas =?iso-8859-1?Q?Hellstr=F6m?= , Rodrigo Vivi , Jarkko Nikula , David Airlie , Simona Vetter , Mika Westerberg , Jan Dabros , Andi Shyti , Raag Jadav , "Tauro, Riana" , "Adatrao, Srinivasa" , "Michael J. Ruhl" , intel-xe@lists.freedesktop.org, linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v4 2/4] drm/xe: Support for I2C attached MCUs Message-ID: References: <20250626135610.299943-1-heikki.krogerus@linux.intel.com> <20250626135610.299943-3-heikki.krogerus@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - c/o Alberga Business Park, 6 krs, Bertel Jungin Aukio 5, 02600 Espoo X-BeenThere: intel-xe@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel Xe graphics driver List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-xe-bounces@lists.freedesktop.org Sender: "Intel-xe" On Fri, Jun 27, 2025 at 11:56:51AM +0300, Heikki Krogerus wrote: > On Thu, Jun 26, 2025 at 05:21:02PM +0300, Andy Shevchenko wrote: > > On Thu, Jun 26, 2025 at 04:56:07PM +0300, Heikki Krogerus wrote: ... > > > + regmap = devm_regmap_init(i2c->drm_dev, NULL, i2c, &i2c_regmap_config); > > > > Use of i2c->drm_dev makes harder to maintain and understand the code. > > Managed resources should be carefully attached to the correct device, > > otherwise it's inevitable object lifetime related issues. > > > > With > > > > struct device *dev = xe->drm.dev; > > > > and using local dev, it becomes easier to get and avoid such subtle mistakes. > > I have to disagree with you on this one. Local dev pointers create > problems because of the assumption that there is only a single device > in the function to deal with (especially if they are named "dev"), > which is almost never the case - this function is no exception. Hmm... In my experience more than 70% of the drivers are okay with this approach as they do *not* use multiple device pointers. Usually, if we take IIO for the example, they have a physical device, which is depicted by the local variable named 'dev' and a Linux IIO device, which is container that has struct device inside, but it's not used explicitly, IIO APIs use the pointer to the container (and of course its name differs). That said, it seems we have different experience (you are most likely talking about USB cases, where that is indeed quite complicated already). > But I'll add the local variable as you requested - I'll just name it > carefully. Sure. ... > This kinda related but off topic. IMO in cases like this the regmap > should be assigned to the child device that is being created instead > of the parent device. That is currently prevented by the current > regmap API - the device has to be fully registered before the regmap > can be assigned (and I'm not referring to the resource managed devm_* > API), but I'm not convinced that it has to be like that. The problem > is that the parent device may have multiple child devices that each > need a dedicated regmag. So just as a note to self: check if we can > improve the regmap API. I'm not sure I follow. The (some of) MFD drivers, for instance, work with many children and one regmapi that is split over the devices. I don't see any issue with that. The problem is when one tries to mix regmap and non-regmap approaches in the same (big) driver. That's a road to mine field. -- With Best Regards, Andy Shevchenko