Make this driver DT-independent by calling the freshly created helpers,
which reduce boilerplate and open the door for otherwise use cases. No
functional changes for DT based systems.
Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
---
drivers/gpu/drm/bridge/ite-it66121.c | 57 +++++++++++++++++-----------
1 file changed, 35 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
index 925e42f46cd8..688dc1830654 100644
--- a/drivers/gpu/drm/bridge/ite-it66121.c
+++ b/drivers/gpu/drm/bridge/ite-it66121.c
@@ -15,7 +15,6 @@
#include <linux/bitfield.h>
#include <linux/property.h>
#include <linux/regmap.h>
-#include <linux/of_graph.h>
#include <linux/gpio/consumer.h>
#include <linux/pinctrl/consumer.h>
#include <linux/regulator/consumer.h>
@@ -1480,7 +1479,7 @@ static int it66121_audio_codec_init(struct it66121_ctx *ctx, struct device *dev)
dev_dbg(dev, "%s\n", __func__);
- if (!of_property_read_bool(dev->of_node, "#sound-dai-cells")) {
+ if (!fwnode_property_present(dev_fwnode(dev), "#sound-dai-cells")) {
dev_info(dev, "No \"#sound-dai-cells\", no audio\n");
return 0;
}
@@ -1503,13 +1502,36 @@ static const char * const it66121_supplies[] = {
"vcn33", "vcn18", "vrf12"
};
+static int it66121_read_bus_width(struct fwnode_handle *fwnode, u32 *bus_width)
+{
+ struct fwnode_handle *endpoint;
+ u32 val;
+ int ret;
+
+ endpoint = fwnode_graph_get_endpoint_by_id(fwnode, 0, 0, 0);
+ if (!endpoint)
+ return -EINVAL;
+
+ ret = fwnode_property_read_u32(endpoint, "bus-width", &val);
+ fwnode_handle_put(endpoint);
+ if (ret)
+ return ret;
+
+ if (val != 12 && val != 24)
+ return -EINVAL;
+
+ *bus_width = val;
+
+ return 0;
+}
+
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;
+ } else if (!ctx->next_bridge) {
dev_dbg(ctx->dev, "Next bridge not found, deferring probe\n");
return -EPROBE_DEFER;
}
@@ -1577,8 +1590,8 @@ static int it66121_probe(struct i2c_client *client)
return -ENODEV;
}
+ drm_bridge_set_node(&ctx->bridge, fwnode);
ctx->bridge.funcs = &it66121_bridge_funcs;
- ctx->bridge.of_node = dev->of_node;
ctx->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
ctx->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID;
if (client->irq > 0) {
--
2.34.1On Tue, Apr 23, 2024 at 03:19:01AM +0800, Sui Jingfeng wrote:
> Make this driver DT-independent by calling the freshly created helpers,
> which reduce boilerplate and open the door for otherwise use cases. No
> functional changes for DT based systems.
>
> Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
> ---
> drivers/gpu/drm/bridge/ite-it66121.c | 57 +++++++++++++++++-----------
> 1 file changed, 35 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ite-it66121.c b/drivers/gpu/drm/bridge/ite-it66121.c
> index 925e42f46cd8..688dc1830654 100644
> --- a/drivers/gpu/drm/bridge/ite-it66121.c
> +++ b/drivers/gpu/drm/bridge/ite-it66121.c
> @@ -15,7 +15,6 @@
> #include <linux/bitfield.h>
> #include <linux/property.h>
> #include <linux/regmap.h>
> -#include <linux/of_graph.h>
> #include <linux/gpio/consumer.h>
> #include <linux/pinctrl/consumer.h>
> #include <linux/regulator/consumer.h>
> @@ -1480,7 +1479,7 @@ static int it66121_audio_codec_init(struct it66121_ctx *ctx, struct device *dev)
>
> dev_dbg(dev, "%s\n", __func__);
>
> - if (!of_property_read_bool(dev->of_node, "#sound-dai-cells")) {
> + if (!fwnode_property_present(dev_fwnode(dev), "#sound-dai-cells")) {
> dev_info(dev, "No \"#sound-dai-cells\", no audio\n");
> return 0;
> }
> @@ -1503,13 +1502,36 @@ static const char * const it66121_supplies[] = {
> "vcn33", "vcn18", "vrf12"
> };
>
> +static int it66121_read_bus_width(struct fwnode_handle *fwnode, u32 *bus_width)
> +{
> + struct fwnode_handle *endpoint;
> + u32 val;
> + int ret;
> +
> + endpoint = fwnode_graph_get_endpoint_by_id(fwnode, 0, 0, 0);
> + if (!endpoint)
> + return -EINVAL;
> +
> + ret = fwnode_property_read_u32(endpoint, "bus-width", &val);
> + fwnode_handle_put(endpoint);
> + if (ret)
> + return ret;
> +
> + if (val != 12 && val != 24)
> + return -EINVAL;
> +
> + *bus_width = val;
> +
> + return 0;
> +}
Ideally this should come as two patches. First patch extracts the
function, second patch changes the driver to use fwnode / property API.
> +
> 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.
> + } else if (!ctx->next_bridge) {
> dev_dbg(ctx->dev, "Next bridge not found, deferring probe\n");
> return -EPROBE_DEFER;
> }
> @@ -1577,8 +1590,8 @@ static int it66121_probe(struct i2c_client *client)
> return -ENODEV;
> }
>
> + drm_bridge_set_node(&ctx->bridge, fwnode);
> ctx->bridge.funcs = &it66121_bridge_funcs;
> - ctx->bridge.of_node = dev->of_node;
> ctx->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
> ctx->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID;
> if (client->irq > 0) {
> --
> 2.34.1
>
--
With best wishes
Dmitry
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
© 2016 - 2026 Red Hat, Inc.