From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-187.mta1.migadu.com (out-187.mta1.migadu.com [95.215.58.187]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A51096024B for ; Wed, 15 May 2024 10:35:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=95.215.58.187 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1715769307; cv=none; b=b4eGGa+fCrJIqevgVdnyLKgDd8M+211Tz0FjOdeqv6HXqOeOpyGxPFr+mRPj2V6bP2a59QEKSadQns3DCTU1eEwdmqRO07lWwcs/AjsUMkqrPiONs1MzoAN8AcpZbz0pGdjPpUbJNrPJI19wWrDMdFZVkl+kB2kFuJexCgBY14A= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1715769307; c=relaxed/simple; bh=Q51DgOo0U4wJrOPU3LVFgYiKGscX5hlbKql6HDNZNEQ=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=puKcBLe6xA5Rt//MRe36X+hb0qJmi1uz8ZBBQ9izqe+ku0pZRKhTuDCaPm/p6X+7W3MPKQxH567Y19QHgcgU/VF01gc+tHI48taa4jB6rEPx/4kQ29yixVl8HZ0nOvvjnq9AOtbh1mQHX6imbYVHY7mhygPdCErX14ndymW5rGk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=aDbI0B6B; arc=none smtp.client-ip=95.215.58.187 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="aDbI0B6B" Message-ID: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1715769303; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=LkdIypPkNo7ZCkUM8QaRa9DJEq1BNKqggZXsE+KZK30=; b=aDbI0B6Ba1g2/Mdo8UvEU9SQZvcC9rJVNzq/91r9EBWX9kEcdg3WGr4Vlio7LW3IVDIBGf NURuIB5mCVVPXpKp6Zo5w4ZZwD7kSdgHs/70xlaglM1Fb2/LpQYKu6tAd55A16rWO18daV JbJkyITxF7wDEaT9gYxY4m/rbb4KZtY= Date: Wed, 15 May 2024 18:34:59 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Subject: Re: [PATCH 1/2] drm/bridge: Support finding bridge with struct device To: Jani Nikula , Neil Armstrong , Maxime Ripard , Dmitry Baryshkov Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org References: <20240514154045.309925-1-sui.jingfeng@linux.dev> <20240514154045.309925-2-sui.jingfeng@linux.dev> <87v83fct2e.fsf@intel.com> <87pltncqtg.fsf@intel.com> Content-Language: en-US, en-AU X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Sui Jingfeng In-Reply-To: <87pltncqtg.fsf@intel.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Migadu-Flow: FLOW_OUT Hi, On 5/15/24 18:28, Jani Nikula wrote: > On Wed, 15 May 2024, Sui Jingfeng wrote: >> Hi, >> >> >> On 5/15/24 17:39, Jani Nikula wrote: >>> On Tue, 14 May 2024, Sui Jingfeng wrote: >>>> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c >>>> index 584d109330ab..1928d9d0dd3c 100644 >>>> --- a/drivers/gpu/drm/drm_bridge.c >>>> +++ b/drivers/gpu/drm/drm_bridge.c >>>> @@ -213,6 +213,23 @@ void drm_bridge_add(struct drm_bridge *bridge) >>>> } >>>> EXPORT_SYMBOL(drm_bridge_add); >>>> >>>> +/** >>>> + * drm_bridge_add_with_dev - add the given bridge to the global bridge list >>>> + * >>>> + * @bridge: bridge control structure >>>> + * @dev: pointer to the kernel device that this bridge is backed. >>>> + */ >>>> +void drm_bridge_add_with_dev(struct drm_bridge *bridge, struct device *dev) >>>> +{ >>>> + if (dev) { >>>> + bridge->kdev = dev; >>>> + bridge->of_node = dev->of_node; >>>> + } >>>> + >>>> + drm_bridge_add(bridge); >>>> +} >>>> +EXPORT_SYMBOL_GPL(drm_bridge_add_with_dev); >>> >>> I don't actually have an opinion on whether the dev parameter is useful >>> or not. >>> >>> But please don't add a drm_bridge_add_with_dev() and then convert more >>> than half the drm_bridge_add() users to that. Please just add a struct >>> device *dev parameter to drm_bridge_add(), and pass NULL if it's not >>> relevant. >>> >> >> To be honest, previously, I'm just do it exactly same as the way you >> told me here. But I'm exhausted and finally give up. >> >> Because this is again need me to modify *all* callers of >> drm_bridge_add(), not only those bridges in drm/bridge/, but also >> bridge instances in various KMS drivers. >> >> However, their some exceptions just don't fit! >> >> For example, the imx/imx8qxp-pixel-combiner.c just don't fit our >> simple model. Our helper function assume that one device backing >> one drm_bridge instance (1 to 1). Yet, that driver backing two or >> more bridges with one platform device (1 to 2, 1 to 3, ..., ). >> Hence, the imx/imx8qxp-pixel-combiner.c just can't use >> drm_bridge_add_with_dev(). >> >> The aux_hpd_bridge.c is also bad, it store the of_node of struct device >> at the .platform_data member of the struct device. > > Like I said, "pass NULL if it's not relevant." OK, good idea. > "_add_with_dev" is a terrible function name. > > What if you need to add another parameter later? Add _add_with_foo and > _add_with_dev_and_foo variants? Yes, you are right, I'll back give another try then. > BR, > Jani. > > -- Best regards Sui