Re: [PATCH v3 2/2] display/drm/bridge: TC358775 DSI/LVDS driver

[Sam Ravnborg <sam@ravnborg.org>]

Hi Vinay.

> +
> +static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id)
> +{
> +	struct device *dev = &client->dev;
> +	struct drm_panel *panel;
> +	struct tc_data *tc;
> +	int ret;
> +
> +	tc = devm_kzalloc(dev, sizeof(*tc), GFP_KERNEL);
> +	if (!tc)
> +		return -ENOMEM;
> +
> +	tc->dev = dev;
> +	tc->i2c = client;
> +
> +	ret = drm_of_find_panel_or_bridge(dev->of_node, TC358775_LVDS_OUT0,
> +					  0, &panel, NULL);
> +	if (ret < 0)
> +		return ret;
> +	if (!panel)
> +		return -ENODEV;
> +
> +	panel->connector_type = DRM_MODE_CONNECTOR_LVDS;

The panel is responsible for specifying the connector_type.
It is wrong to let users of panel set it.

Fix the panel and not the bridge driver.


> +
> +	tc->panel_bridge = devm_drm_panel_bridge_add(dev, panel);
> +	if (IS_ERR(tc->panel_bridge))
> +		return PTR_ERR(tc->panel_bridge);
> +
> +	ret = tc358775_parse_dt(dev->of_node, tc);
> +	if (ret)
> +		return ret;
> +
> +	tc->vddio = devm_regulator_get(dev, "vddio-supply");
> +	if (IS_ERR(tc->vddio)) {
> +		ret = PTR_ERR(tc->vddio);
> +		dev_err(dev, "vddio-supply not found\n");
> +		return ret;
> +	}
> +
> +	tc->vdd = devm_regulator_get(dev, "vdd-supply");
> +	if (IS_ERR(tc->vdd)) {
> +		ret = PTR_ERR(tc->vddio);
> +		dev_err(dev, "vdd-supply not found\n");
> +		return ret;
> +	}
> +
> +	tc->stby_gpio = devm_gpiod_get(dev, "stby", GPIOD_OUT_HIGH);
> +	if (IS_ERR(tc->stby_gpio)) {
> +		ret = PTR_ERR(tc->stby_gpio);
> +		dev_err(dev, "cannot get stby-gpio %d\n", ret);
> +		return ret;
> +	}
> +
> +	tc->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(tc->reset_gpio)) {
> +		ret = PTR_ERR(tc->reset_gpio);
> +		dev_err(dev, "cannot get reset-gpios %d\n", ret);
> +		return ret;
> +	}
> +
> +	tc->bridge.funcs = &tc_bridge_funcs;
> +	tc->bridge.of_node = dev->of_node;
> +	drm_bridge_add(&tc->bridge);
> +
> +	i2c_set_clientdata(client, tc);
> +
> +	return 0;
> +}
> +

	Sam