[PATCH v2] drm: bridge: tfp410: Check device ID for I2C-connected TFP410

Neil Armstrong narmstrong at baylibre.com
Tue Mar 10 10:18:28 UTC 2020


On 09/03/2020 21:37, Laurent Pinchart wrote:
> The TFP410 supports configuration through I2C (in which case the
> corresponding DT node is a child of an I2C controller) or through pins
> (in which case the DT node creates a platform device). When I2C access
> to the device is available, read and validate the device ID at probe
> time to ensure that the device is present.
> 
> While at it, sort headers alphabetically.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> This time based on drm-misc-next instead of v5.6-rc5, with conflicts
> resolved.
> 
>  drivers/gpu/drm/bridge/ti-tfp410.c | 134 +++++++++++++++++++++++++----
>  1 file changed, 115 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c
> index 40c4d4a5517b..be8d74ff632b 100644
> --- a/drivers/gpu/drm/bridge/ti-tfp410.c
> +++ b/drivers/gpu/drm/bridge/ti-tfp410.c
> @@ -9,6 +9,7 @@
>  #include <linux/module.h>
>  #include <linux/of_graph.h>
>  #include <linux/platform_device.h>
> +#include <linux/regmap.h>
>  #include <linux/workqueue.h>
>  
>  #include <drm/drm_atomic_helper.h>
> @@ -26,6 +27,7 @@ struct tfp410 {
>  	u32			bus_format;
>  	struct delayed_work	hpd_work;
>  	struct gpio_desc	*powerdown;
> +	struct regmap		*regmap;
>  
>  	struct drm_bridge_timings timings;
>  	struct drm_bridge	*next_bridge;
> @@ -213,7 +215,7 @@ static const struct drm_bridge_timings tfp410_default_timings = {
>  	.hold_time_ps = 1300,
>  };
>  
> -static int tfp410_parse_timings(struct tfp410 *dvi, bool i2c)
> +static int tfp410_parse_timings(struct tfp410 *dvi)
>  {
>  	struct drm_bridge_timings *timings = &dvi->timings;
>  	struct device_node *ep;
> @@ -224,7 +226,7 @@ static int tfp410_parse_timings(struct tfp410 *dvi, bool i2c)
>  	/* Start with defaults. */
>  	*timings = tfp410_default_timings;
>  
> -	if (i2c)
> +	if (dvi->regmap)
>  		/*
>  		 * In I2C mode timings are configured through the I2C interface.
>  		 * As the driver doesn't support I2C configuration yet, we just
> @@ -283,10 +285,10 @@ static int tfp410_parse_timings(struct tfp410 *dvi, bool i2c)
>  	return 0;
>  }
>  
> -static int tfp410_init(struct device *dev, bool i2c)
> +static int tfp410_init(struct tfp410 *dvi)
>  {
>  	struct device_node *node;
> -	struct tfp410 *dvi;
> +	struct device *dev = dvi->dev;
>  	int ret;
>  
>  	if (!dev->of_node) {
> @@ -294,11 +296,6 @@ static int tfp410_init(struct device *dev, bool i2c)
>  		return -ENXIO;
>  	}
>  
> -	dvi = devm_kzalloc(dev, sizeof(*dvi), GFP_KERNEL);
> -	if (!dvi)
> -		return -ENOMEM;
> -
> -	dvi->dev = dev;
>  	dev_set_drvdata(dev, dvi);
>  
>  	dvi->bridge.funcs = &tfp410_bridge_funcs;
> @@ -306,7 +303,7 @@ static int tfp410_init(struct device *dev, bool i2c)
>  	dvi->bridge.timings = &dvi->timings;
>  	dvi->bridge.type = DRM_MODE_CONNECTOR_DVID;
>  
> -	ret = tfp410_parse_timings(dvi, i2c);
> +	ret = tfp410_parse_timings(dvi);
>  	if (ret)
>  		return ret;
>  
> @@ -346,7 +343,15 @@ static int tfp410_fini(struct device *dev)
>  
>  static int tfp410_probe(struct platform_device *pdev)
>  {
> -	return tfp410_init(&pdev->dev, false);
> +	struct tfp410 *dvi;
> +
> +	dvi = devm_kzalloc(&pdev->dev, sizeof(*dvi), GFP_KERNEL);
> +	if (!dvi)
> +		return -ENOMEM;
> +
> +	dvi->dev = &pdev->dev;
> +
> +	return tfp410_init(dvi);
>  }
>  
>  static int tfp410_remove(struct platform_device *pdev)
> @@ -370,20 +375,111 @@ static struct platform_driver tfp410_platform_driver = {
>  };
>  
>  #if IS_ENABLED(CONFIG_I2C)
> -/* There is currently no i2c functionality. */
> +
> +#define TFP410_VEN_ID_LO		0x00
> +#define TFP410_VEN_ID_HI		0x01
> +#define TFP410_DEV_ID_LO		0x02
> +#define TFP410_DEV_ID_HI		0x03
> +#define TFP410_REV_ID			0x04
> +#define TFP410_CTL_1_MODE		0x08
> +#define TFP410_CTL_2_MODE		0x09
> +#define TFP410_CTL_3_MODE		0x0a
> +#define TFP410_CFG			0x0b
> +#define TFP410_DE_DLY			0x32
> +#define TFP410_DE_CTL			0x33
> +#define TFP410_DE_TOP			0x34
> +#define TFP410_DE_CNT_LO		0x36
> +#define TFP410_DE_CNT_HI		0x37
> +#define TFP410_DE_LIN_LO		0x38
> +#define TFP410_DE_LIN_HI		0x39
> +#define TFP410_H_RES_LO			0x3a
> +#define TFP410_H_RES_HI			0x3b
> +#define TFP410_V_RES_LO			0x3c
> +#define TFP410_V_RES_HI			0x3d
> +
> +static const struct regmap_config tfp410_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +
> +	.max_register = 0x3d,
> +	.wr_table = &(const struct regmap_access_table) {
> +		.yes_ranges = (const struct regmap_range[]) {
> +			{
> +				.range_min = TFP410_CTL_1_MODE,
> +				.range_max = TFP410_DE_LIN_HI,
> +			},
> +		},
> +		.n_yes_ranges = 1,
> +	},
> +};
> +
> +static int tfp410_check_version(struct tfp410 *dvi)
> +{
> +	unsigned int value;
> +	u16 vendor_id;
> +	u16 device_id;
> +	u8 revision_id;
> +	int ret;
> +
> +	ret = regmap_read(dvi->regmap, TFP410_VEN_ID_LO, &value);
> +	if (ret < 0)
> +		return ret;
> +	vendor_id = value;
> +
> +	ret = regmap_read(dvi->regmap, TFP410_VEN_ID_HI, &value);
> +	if (ret < 0)
> +		return ret;
> +	vendor_id |= value << 8;
> +
> +	ret = regmap_read(dvi->regmap, TFP410_DEV_ID_LO, &value);
> +	if (ret < 0)
> +		return ret;
> +	device_id = value;
> +
> +	ret = regmap_read(dvi->regmap, TFP410_DEV_ID_HI, &value);
> +	if (ret < 0)
> +		return ret;
> +	device_id |= value << 8;
> +
> +	ret = regmap_read(dvi->regmap, TFP410_REV_ID, &value);
> +	if (ret < 0)
> +		return ret;
> +	revision_id = value;
> +
> +	if (vendor_id != 0x014c || device_id != 0x0410) {
> +		dev_err(dvi->dev, "invalid device ID %04x:%04x\n",
> +			vendor_id, device_id);
> +		return -ENODEV;
> +	}
> +
> +	dev_info(dvi->dev, "Found TFP410 revision 0x%02x\n", revision_id);
> +
> +	return 0;
> +}
> +
>  static int tfp410_i2c_probe(struct i2c_client *client,
>  			    const struct i2c_device_id *id)
>  {
> -	int reg;
> +	struct tfp410 *dvi;
> +	int ret;
>  
> -	if (!client->dev.of_node ||
> -	    of_property_read_u32(client->dev.of_node, "reg", &reg)) {
> -		dev_err(&client->dev,
> -			"Can't get i2c reg property from device-tree\n");
> -		return -ENXIO;
> +	dvi = devm_kzalloc(&client->dev, sizeof(*dvi), GFP_KERNEL);
> +	if (!dvi)
> +		return -ENOMEM;
> +
> +	dvi->dev = &client->dev;
> +
> +	dvi->regmap = devm_regmap_init_i2c(client, &tfp410_regmap_config);
> +	if (IS_ERR(dvi->regmap))
> +		return PTR_ERR(dvi->regmap);
> +
> +	ret = tfp410_check_version(dvi);
> +	if (ret < 0) {
> +		dev_err(dvi->dev, "failed to read device ID (%d)\n", ret);
> +		return ret;
>  	}
>  
> -	return tfp410_init(&client->dev, true);
> +	return tfp410_init(dvi);
>  }
>  
>  static int tfp410_i2c_remove(struct i2c_client *client)
> 

Reviewed-by: Neil Armstrong <narmstrong at baylibre.com>

Neil


More information about the dri-devel mailing list