Re: [PATCH v2] drm: bridge: tfp410: Check device ID for I2C-connected TFP410
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
> ---
> 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
> #include
> #include
> +#include
> #include
>
> #include
> @@ -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 = >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 = _bridge_funcs;
> @@ -306,7 +303,7 @@ static int tfp410_init(struct device *dev, bool i2c)
> dvi->bridge.timings = >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(>dev, false);
> + struct tfp410 *dvi;
> +
> + dvi = devm_kzalloc(>dev, sizeof(*dvi), GFP_KERNEL);
> + if (!dvi)
> + return -ENOMEM;
> +
> + dvi->dev = >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_ID0x04
> +#define TFP410_CTL_1_MODE0x08
> +#define TFP410_CTL_2_MODE0x09
> +#define TFP410_CTL_3_MODE0x0a
> +#define TFP410_CFG 0x0b
> +#define TFP410_DE_DLY0x32
> +#define TFP410_DE_CTL0x33
> +#define TFP410_DE_TOP0x34
> +#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 =
[PATCH v2] drm: bridge: tfp410: Check device ID for I2C-connected TFP410
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
---
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
#include
#include
+#include
#include
#include
@@ -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 = >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 = _bridge_funcs;
@@ -306,7 +303,7 @@ static int tfp410_init(struct device *dev, bool i2c)
dvi->bridge.timings = >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(>dev, false);
+ struct tfp410 *dvi;
+
+ dvi = devm_kzalloc(>dev, sizeof(*dvi), GFP_KERNEL);
+ if (!dvi)
+ return -ENOMEM;
+
+ dvi->dev = >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_LO0x3a
+#define TFP410_H_RES_HI0x3b
+#define TFP410_V_RES_LO0x3c
+#define TFP410_V_RES_HI0x3d
+
+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)
+{
+
