Received-SPF: pass (google.com: domain of linux-kernel+bounces-161048-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <fc9132ec-68ae-4d28-afe8-0fc6ab2eec8c@linux.dev>
Date: Sat, 27 Apr 2024 21:11:47 +0800
Precedence: bulk
MIME-Version: 1.0
Subject: Re: [PATCH v4 7/9] drm-bridge: it66121: Use fwnode API to acquire
 device properties
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Neil Armstrong <neil.armstrong@linaro.org>, Robert Foss
 <rfoss@kernel.org>, Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
 Andrzej Hajda <andrzej.hajda@intel.com>, Jonas Karlman <jonas@kwiboo.se>,
 Jernej Skrabec <jernej.skrabec@gmail.com>, Maxime Ripard
 <mripard@kernel.org>, Thomas Zimmermann <tzimmermann@suse.de>,
 David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>,
 Phong LE <ple@baylibre.com>, dri-devel@lists.freedesktop.org,
 linux-kernel@vger.kernel.org
References: <20240422191903.255642-1-sui.jingfeng@linux.dev>
 <20240422191903.255642-8-sui.jingfeng@linux.dev>
 <ugo62mcrvo5csp7umzvn3jhffh625agnjr3rtujnbgm7gxvgtr@re4q2xg46iqn>
Content-Language: en-US
From: Sui Jingfeng <sui.jingfeng@linux.dev>
In-Reply-To: <ugo62mcrvo5csp7umzvn3jhffh625agnjr3rtujnbgm7gxvgtr@re4q2xg46iqn>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

Hi,


On 2024/4/23 04:06, Dmitry Baryshkov wrote:
>> +
>>   static int it66121_probe(struct i2c_client *client)
>>   {
>>   	u32 revision_id, vendor_ids[2] = { 0 }, device_ids[2] = { 0 };
>> -	struct device_node *ep;
>>   	int ret;
>>   	struct it66121_ctx *ctx;
>>   	struct device *dev = &client->dev;
>> +	struct fwnode_handle *fwnode = dev_fwnode(dev);
>>   
>>   	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
>>   		dev_err(dev, "I2C check functionality failed.\n");
>> @@ -1520,29 +1542,20 @@ static int it66121_probe(struct i2c_client *client)
>>   	if (!ctx)
>>   		return -ENOMEM;
>>   
>> -	ep = of_graph_get_endpoint_by_regs(dev->of_node, 0, 0);
>> -	if (!ep)
>> -		return -EINVAL;
>> -
>>   	ctx->dev = dev;
>>   	ctx->client = client;
>>   	ctx->info = i2c_get_match_data(client);
>>   
>> -	of_property_read_u32(ep, "bus-width", &ctx->bus_width);
>> -	of_node_put(ep);
>> -
>> -	if (ctx->bus_width != 12 && ctx->bus_width != 24)
>> -		return -EINVAL;
>> -
>> -	ep = of_graph_get_remote_node(dev->of_node, 1, -1);
>> -	if (!ep) {
>> -		dev_err(ctx->dev, "The endpoint is unconnected\n");
>> -		return -EINVAL;
>> -	}
>> +	ret = it66121_read_bus_width(fwnode, &ctx->bus_width);
>> +	if (ret)
>> +		return ret;
>>   
>> -	ctx->next_bridge = of_drm_find_bridge(ep);
>> -	of_node_put(ep);
>> -	if (!ctx->next_bridge) {
>> +	ctx->next_bridge = drm_bridge_find_next_bridge_by_fwnode(fwnode, 1);
>> +	if (IS_ERR(ctx->next_bridge)) {
>> +		ret = PTR_ERR(ctx->next_bridge);
>> +		dev_err(dev, "Error in founding the next bridge: %d\n", ret);
>> +		return ret;
> return dev_err_probe(dev, ret, "msg"), if your function doesn't do this.
> If it does, just return ret.


My drm_bridge_find_next_bridge_by_fwnode() function won't return -EPROBE_DEFER
this is known for sure. As a//prior(priori) knowledge. Calling the dev_err_probe()
just introduce extra overhead. It is useless to use dev_err_probe() here.
  

-- 
Best regards,
Sui