All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/gma500: Pass GPIO for Intel MID using descriptors
@ 2019-12-06  9:43 ` Linus Walleij
  0 siblings, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2019-12-06  9:43 UTC (permalink / raw)
  To: dri-devel, Maarten Lankhorst, Maxime Ripard, Sean Paul
  Cc: Daniel Stone, Daniel Vetter, Linus Walleij, Patrik Jakobsson,
	Andy Shevchenko, linux-arm-kernel

The GMA500 driver is using the legacy GPIO API to fetch
three optional display control GPIO lines from the SFI
description used by the Medfield platform.

Switch this over to use GPIO descriptors and delete the
custom platform data.

We create three new static locals in the tc35876x bridge
code but it is hardly any worse than the I2C client static
local already there: I tried first to move it to the DRM
driver state container but there are workarounds for
probe order in the code so I just stayed off it, as the
result is unpredictable.

People wanting to do a more throrugh and proper cleanup
of the GMA500 driver can work on top of this, I can't
solve much more since I don't have access to the hardware,
I can only attempt to tidy up my GPIO corner.

Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
Cc: Daniel Stone <daniels@collabora.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
If someone can test this on the target platform I'd be
happy.
---
 .../intel-mid/device_libs/platform_tc35876x.c | 26 ++++--
 drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c    | 88 +++++++------------
 include/linux/platform_data/tc35876x.h        | 11 ---
 3 files changed, 51 insertions(+), 74 deletions(-)
 delete mode 100644 include/linux/platform_data/tc35876x.h

diff --git a/arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c b/arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c
index 44d1f884c3d3..139738bbdd36 100644
--- a/arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c
+++ b/arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c
@@ -6,21 +6,31 @@
  * Author: Sathyanarayanan Kuppuswamy <sathyanarayanan.kuppuswamy@intel.com>
  */
 
-#include <linux/gpio.h>
-#include <linux/platform_data/tc35876x.h>
+#include <linux/gpio/machine.h>
 #include <asm/intel-mid.h>
 
+static struct gpiod_lookup_table tc35876x_gpio_table = {
+	.dev_id	= "i2c_disp_brig",
+	.table	= {
+		GPIO_LOOKUP("0000:00:0c.0", -1, "bridge-reset", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("0000:00:0c.0", -1, "bl-en", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("0000:00:0c.0", -1, "vadd", GPIO_ACTIVE_HIGH),
+		{ },
+	},
+};
+
 /*tc35876x DSI_LVDS bridge chip and panel platform data*/
 static void *tc35876x_platform_data(void *data)
 {
-	static struct tc35876x_platform_data pdata;
+	struct gpiod_lookup_table *table = &tc35876x_gpio_table;
+	struct gpiod_lookup *lookup = table->table;
 
-	/* gpio pins set to -1 will not be used by the driver */
-	pdata.gpio_bridge_reset = get_gpio_by_name("LCMB_RXEN");
-	pdata.gpio_panel_bl_en = get_gpio_by_name("6S6P_BL_EN");
-	pdata.gpio_panel_vadd = get_gpio_by_name("EN_VREG_LCD_V3P3");
+	lookup[0].chip_hwnum = get_gpio_by_name("LCMB_RXEN");
+	lookup[1].chip_hwnum = get_gpio_by_name("6S6P_BL_EN");
+	lookup[2].chip_hwnum = get_gpio_by_name("EN_VREG_LCD_V3P3");
+	gpiod_add_lookup_table(table);
 
-	return &pdata;
+	return NULL;
 }
 
 static const struct devs_id tc35876x_dev_id __initconst = {
diff --git a/drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c b/drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c
index 7de3ce637c7f..9e8224456ea2 100644
--- a/drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c
+++ b/drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c
@@ -25,7 +25,7 @@
 #include <linux/delay.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/platform_data/tc35876x.h>
+#include <linux/gpio/consumer.h>
 
 #include <asm/intel_scu_ipc.h>
 
@@ -36,6 +36,11 @@
 
 static struct i2c_client *tc35876x_client;
 static struct i2c_client *cmi_lcd_i2c_client;
+/* Panel GPIOs */
+static struct gpio_desc *bridge_reset;
+static struct gpio_desc *bridge_bl_enable;
+static struct gpio_desc *backlight_voltage;
+
 
 #define FLD_MASK(start, end)	(((1 << ((start) - (end) + 1)) - 1) << (end))
 #define FLD_VAL(val, start, end) (((val) << (end)) & FLD_MASK(start, end))
@@ -316,27 +321,23 @@ static int tc35876x_regr(struct i2c_client *client, u16 reg, u32 *value)
 
 void tc35876x_set_bridge_reset_state(struct drm_device *dev, int state)
 {
-	struct tc35876x_platform_data *pdata;
-
 	if (WARN(!tc35876x_client, "%s called before probe", __func__))
 		return;
 
 	dev_dbg(&tc35876x_client->dev, "%s: state %d\n", __func__, state);
 
-	pdata = dev_get_platdata(&tc35876x_client->dev);
-
-	if (pdata->gpio_bridge_reset == -1)
+	if (!bridge_reset)
 		return;
 
 	if (state) {
-		gpio_set_value_cansleep(pdata->gpio_bridge_reset, 0);
+		gpiod_set_value_cansleep(bridge_reset, 0);
 		mdelay(10);
 	} else {
 		/* Pull MIPI Bridge reset pin to Low */
-		gpio_set_value_cansleep(pdata->gpio_bridge_reset, 0);
+		gpiod_set_value_cansleep(bridge_reset, 0);
 		mdelay(20);
 		/* Pull MIPI Bridge reset pin to High */
-		gpio_set_value_cansleep(pdata->gpio_bridge_reset, 1);
+		gpiod_set_value_cansleep(bridge_reset, 1);
 		mdelay(40);
 	}
 }
@@ -510,25 +511,20 @@ void tc35876x_brightness_control(struct drm_device *dev, int level)
 
 void tc35876x_toshiba_bridge_panel_off(struct drm_device *dev)
 {
-	struct tc35876x_platform_data *pdata;
-
 	if (WARN(!tc35876x_client, "%s called before probe", __func__))
 		return;
 
 	dev_dbg(&tc35876x_client->dev, "%s\n", __func__);
 
-	pdata = dev_get_platdata(&tc35876x_client->dev);
-
-	if (pdata->gpio_panel_bl_en != -1)
-		gpio_set_value_cansleep(pdata->gpio_panel_bl_en, 0);
+	if (bridge_bl_enable)
+		gpiod_set_value_cansleep(bridge_bl_enable, 0);
 
-	if (pdata->gpio_panel_vadd != -1)
-		gpio_set_value_cansleep(pdata->gpio_panel_vadd, 0);
+	if (backlight_voltage)
+		gpiod_set_value_cansleep(backlight_voltage, 0);
 }
 
 void tc35876x_toshiba_bridge_panel_on(struct drm_device *dev)
 {
-	struct tc35876x_platform_data *pdata;
 	struct drm_psb_private *dev_priv = dev->dev_private;
 
 	if (WARN(!tc35876x_client, "%s called before probe", __func__))
@@ -536,10 +532,8 @@ void tc35876x_toshiba_bridge_panel_on(struct drm_device *dev)
 
 	dev_dbg(&tc35876x_client->dev, "%s\n", __func__);
 
-	pdata = dev_get_platdata(&tc35876x_client->dev);
-
-	if (pdata->gpio_panel_vadd != -1) {
-		gpio_set_value_cansleep(pdata->gpio_panel_vadd, 1);
+	if (backlight_voltage) {
+		gpiod_set_value_cansleep(backlight_voltage, 1);
 		msleep(260);
 	}
 
@@ -571,8 +565,8 @@ void tc35876x_toshiba_bridge_panel_on(struct drm_device *dev)
 				"i2c write failed (%d)\n", ret);
 	}
 
-	if (pdata->gpio_panel_bl_en != -1)
-		gpio_set_value_cansleep(pdata->gpio_panel_bl_en, 1);
+	if (bridge_bl_enable)
+		gpiod_set_value_cansleep(bridge_bl_enable, 1);
 
 	tc35876x_brightness_control(dev, dev_priv->brightness_adjusted);
 }
@@ -635,8 +629,6 @@ static int tc35876x_get_panel_info(struct drm_device *dev, int pipe,
 static int tc35876x_bridge_probe(struct i2c_client *client,
 				const struct i2c_device_id *id)
 {
-	struct tc35876x_platform_data *pdata;
-
 	dev_info(&client->dev, "%s\n", __func__);
 
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
@@ -645,26 +637,23 @@ static int tc35876x_bridge_probe(struct i2c_client *client,
 		return -ENODEV;
 	}
 
-	pdata = dev_get_platdata(&client->dev);
-	if (!pdata) {
-		dev_err(&client->dev, "%s: no platform data\n", __func__);
-		return -ENODEV;
-	}
+	bridge_reset = devm_gpiod_get_optional(&client->dev, "bridge-reset", GPIOD_OUT_LOW);
+	if (IS_ERR(bridge_reset))
+		return PTR_ERR(bridge_reset);
+	if (bridge_reset)
+		gpiod_set_consumer_name(bridge_reset, "tc35876x bridge reset");
 
-	if (pdata->gpio_bridge_reset != -1) {
-		gpio_request(pdata->gpio_bridge_reset, "tc35876x bridge reset");
-		gpio_direction_output(pdata->gpio_bridge_reset, 0);
-	}
-
-	if (pdata->gpio_panel_bl_en != -1) {
-		gpio_request(pdata->gpio_panel_bl_en, "tc35876x panel bl en");
-		gpio_direction_output(pdata->gpio_panel_bl_en, 0);
-	}
+	bridge_bl_enable = devm_gpiod_get_optional(&client->dev, "bl-en", GPIOD_OUT_LOW);
+	if (IS_ERR(bridge_bl_enable))
+		return PTR_ERR(bridge_bl_enable);
+	if (bridge_bl_enable)
+		gpiod_set_consumer_name(bridge_bl_enable, "tc35876x panel bl en");
 
-	if (pdata->gpio_panel_vadd != -1) {
-		gpio_request(pdata->gpio_panel_vadd, "tc35876x panel vadd");
-		gpio_direction_output(pdata->gpio_panel_vadd, 0);
-	}
+	backlight_voltage = devm_gpiod_get_optional(&client->dev, "vadd", GPIOD_OUT_LOW);
+	if (IS_ERR(backlight_voltage))
+		return PTR_ERR(backlight_voltage);
+	if (backlight_voltage)
+		gpiod_set_consumer_name(backlight_voltage, "tc35876x panel vadd");
 
 	tc35876x_client = client;
 
@@ -673,19 +662,8 @@ static int tc35876x_bridge_probe(struct i2c_client *client,
 
 static int tc35876x_bridge_remove(struct i2c_client *client)
 {
-	struct tc35876x_platform_data *pdata = dev_get_platdata(&client->dev);
-
 	dev_dbg(&client->dev, "%s\n", __func__);
 
-	if (pdata->gpio_bridge_reset != -1)
-		gpio_free(pdata->gpio_bridge_reset);
-
-	if (pdata->gpio_panel_bl_en != -1)
-		gpio_free(pdata->gpio_panel_bl_en);
-
-	if (pdata->gpio_panel_vadd != -1)
-		gpio_free(pdata->gpio_panel_vadd);
-
 	tc35876x_client = NULL;
 
 	return 0;
diff --git a/include/linux/platform_data/tc35876x.h b/include/linux/platform_data/tc35876x.h
deleted file mode 100644
index cd6a51c71e7e..000000000000
--- a/include/linux/platform_data/tc35876x.h
+++ /dev/null
@@ -1,11 +0,0 @@
-
-#ifndef _TC35876X_H
-#define _TC35876X_H
-
-struct tc35876x_platform_data {
-	int gpio_bridge_reset;
-	int gpio_panel_bl_en;
-	int gpio_panel_vadd;
-};
-
-#endif /* _TC35876X_H */
-- 
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] drm/gma500: Pass GPIO for Intel MID using descriptors
@ 2019-12-06  9:43 ` Linus Walleij
  0 siblings, 0 replies; 6+ messages in thread
From: Linus Walleij @ 2019-12-06  9:43 UTC (permalink / raw)
  To: dri-devel, Maarten Lankhorst, Maxime Ripard, Sean Paul
  Cc: Daniel Stone, Daniel Vetter, Andy Shevchenko, linux-arm-kernel

The GMA500 driver is using the legacy GPIO API to fetch
three optional display control GPIO lines from the SFI
description used by the Medfield platform.

Switch this over to use GPIO descriptors and delete the
custom platform data.

We create three new static locals in the tc35876x bridge
code but it is hardly any worse than the I2C client static
local already there: I tried first to move it to the DRM
driver state container but there are workarounds for
probe order in the code so I just stayed off it, as the
result is unpredictable.

People wanting to do a more throrugh and proper cleanup
of the GMA500 driver can work on top of this, I can't
solve much more since I don't have access to the hardware,
I can only attempt to tidy up my GPIO corner.

Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
Cc: Daniel Stone <daniels@collabora.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
If someone can test this on the target platform I'd be
happy.
---
 .../intel-mid/device_libs/platform_tc35876x.c | 26 ++++--
 drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c    | 88 +++++++------------
 include/linux/platform_data/tc35876x.h        | 11 ---
 3 files changed, 51 insertions(+), 74 deletions(-)
 delete mode 100644 include/linux/platform_data/tc35876x.h

diff --git a/arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c b/arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c
index 44d1f884c3d3..139738bbdd36 100644
--- a/arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c
+++ b/arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c
@@ -6,21 +6,31 @@
  * Author: Sathyanarayanan Kuppuswamy <sathyanarayanan.kuppuswamy@intel.com>
  */
 
-#include <linux/gpio.h>
-#include <linux/platform_data/tc35876x.h>
+#include <linux/gpio/machine.h>
 #include <asm/intel-mid.h>
 
+static struct gpiod_lookup_table tc35876x_gpio_table = {
+	.dev_id	= "i2c_disp_brig",
+	.table	= {
+		GPIO_LOOKUP("0000:00:0c.0", -1, "bridge-reset", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("0000:00:0c.0", -1, "bl-en", GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP("0000:00:0c.0", -1, "vadd", GPIO_ACTIVE_HIGH),
+		{ },
+	},
+};
+
 /*tc35876x DSI_LVDS bridge chip and panel platform data*/
 static void *tc35876x_platform_data(void *data)
 {
-	static struct tc35876x_platform_data pdata;
+	struct gpiod_lookup_table *table = &tc35876x_gpio_table;
+	struct gpiod_lookup *lookup = table->table;
 
-	/* gpio pins set to -1 will not be used by the driver */
-	pdata.gpio_bridge_reset = get_gpio_by_name("LCMB_RXEN");
-	pdata.gpio_panel_bl_en = get_gpio_by_name("6S6P_BL_EN");
-	pdata.gpio_panel_vadd = get_gpio_by_name("EN_VREG_LCD_V3P3");
+	lookup[0].chip_hwnum = get_gpio_by_name("LCMB_RXEN");
+	lookup[1].chip_hwnum = get_gpio_by_name("6S6P_BL_EN");
+	lookup[2].chip_hwnum = get_gpio_by_name("EN_VREG_LCD_V3P3");
+	gpiod_add_lookup_table(table);
 
-	return &pdata;
+	return NULL;
 }
 
 static const struct devs_id tc35876x_dev_id __initconst = {
diff --git a/drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c b/drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c
index 7de3ce637c7f..9e8224456ea2 100644
--- a/drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c
+++ b/drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c
@@ -25,7 +25,7 @@
 #include <linux/delay.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/platform_data/tc35876x.h>
+#include <linux/gpio/consumer.h>
 
 #include <asm/intel_scu_ipc.h>
 
@@ -36,6 +36,11 @@
 
 static struct i2c_client *tc35876x_client;
 static struct i2c_client *cmi_lcd_i2c_client;
+/* Panel GPIOs */
+static struct gpio_desc *bridge_reset;
+static struct gpio_desc *bridge_bl_enable;
+static struct gpio_desc *backlight_voltage;
+
 
 #define FLD_MASK(start, end)	(((1 << ((start) - (end) + 1)) - 1) << (end))
 #define FLD_VAL(val, start, end) (((val) << (end)) & FLD_MASK(start, end))
@@ -316,27 +321,23 @@ static int tc35876x_regr(struct i2c_client *client, u16 reg, u32 *value)
 
 void tc35876x_set_bridge_reset_state(struct drm_device *dev, int state)
 {
-	struct tc35876x_platform_data *pdata;
-
 	if (WARN(!tc35876x_client, "%s called before probe", __func__))
 		return;
 
 	dev_dbg(&tc35876x_client->dev, "%s: state %d\n", __func__, state);
 
-	pdata = dev_get_platdata(&tc35876x_client->dev);
-
-	if (pdata->gpio_bridge_reset == -1)
+	if (!bridge_reset)
 		return;
 
 	if (state) {
-		gpio_set_value_cansleep(pdata->gpio_bridge_reset, 0);
+		gpiod_set_value_cansleep(bridge_reset, 0);
 		mdelay(10);
 	} else {
 		/* Pull MIPI Bridge reset pin to Low */
-		gpio_set_value_cansleep(pdata->gpio_bridge_reset, 0);
+		gpiod_set_value_cansleep(bridge_reset, 0);
 		mdelay(20);
 		/* Pull MIPI Bridge reset pin to High */
-		gpio_set_value_cansleep(pdata->gpio_bridge_reset, 1);
+		gpiod_set_value_cansleep(bridge_reset, 1);
 		mdelay(40);
 	}
 }
@@ -510,25 +511,20 @@ void tc35876x_brightness_control(struct drm_device *dev, int level)
 
 void tc35876x_toshiba_bridge_panel_off(struct drm_device *dev)
 {
-	struct tc35876x_platform_data *pdata;
-
 	if (WARN(!tc35876x_client, "%s called before probe", __func__))
 		return;
 
 	dev_dbg(&tc35876x_client->dev, "%s\n", __func__);
 
-	pdata = dev_get_platdata(&tc35876x_client->dev);
-
-	if (pdata->gpio_panel_bl_en != -1)
-		gpio_set_value_cansleep(pdata->gpio_panel_bl_en, 0);
+	if (bridge_bl_enable)
+		gpiod_set_value_cansleep(bridge_bl_enable, 0);
 
-	if (pdata->gpio_panel_vadd != -1)
-		gpio_set_value_cansleep(pdata->gpio_panel_vadd, 0);
+	if (backlight_voltage)
+		gpiod_set_value_cansleep(backlight_voltage, 0);
 }
 
 void tc35876x_toshiba_bridge_panel_on(struct drm_device *dev)
 {
-	struct tc35876x_platform_data *pdata;
 	struct drm_psb_private *dev_priv = dev->dev_private;
 
 	if (WARN(!tc35876x_client, "%s called before probe", __func__))
@@ -536,10 +532,8 @@ void tc35876x_toshiba_bridge_panel_on(struct drm_device *dev)
 
 	dev_dbg(&tc35876x_client->dev, "%s\n", __func__);
 
-	pdata = dev_get_platdata(&tc35876x_client->dev);
-
-	if (pdata->gpio_panel_vadd != -1) {
-		gpio_set_value_cansleep(pdata->gpio_panel_vadd, 1);
+	if (backlight_voltage) {
+		gpiod_set_value_cansleep(backlight_voltage, 1);
 		msleep(260);
 	}
 
@@ -571,8 +565,8 @@ void tc35876x_toshiba_bridge_panel_on(struct drm_device *dev)
 				"i2c write failed (%d)\n", ret);
 	}
 
-	if (pdata->gpio_panel_bl_en != -1)
-		gpio_set_value_cansleep(pdata->gpio_panel_bl_en, 1);
+	if (bridge_bl_enable)
+		gpiod_set_value_cansleep(bridge_bl_enable, 1);
 
 	tc35876x_brightness_control(dev, dev_priv->brightness_adjusted);
 }
@@ -635,8 +629,6 @@ static int tc35876x_get_panel_info(struct drm_device *dev, int pipe,
 static int tc35876x_bridge_probe(struct i2c_client *client,
 				const struct i2c_device_id *id)
 {
-	struct tc35876x_platform_data *pdata;
-
 	dev_info(&client->dev, "%s\n", __func__);
 
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
@@ -645,26 +637,23 @@ static int tc35876x_bridge_probe(struct i2c_client *client,
 		return -ENODEV;
 	}
 
-	pdata = dev_get_platdata(&client->dev);
-	if (!pdata) {
-		dev_err(&client->dev, "%s: no platform data\n", __func__);
-		return -ENODEV;
-	}
+	bridge_reset = devm_gpiod_get_optional(&client->dev, "bridge-reset", GPIOD_OUT_LOW);
+	if (IS_ERR(bridge_reset))
+		return PTR_ERR(bridge_reset);
+	if (bridge_reset)
+		gpiod_set_consumer_name(bridge_reset, "tc35876x bridge reset");
 
-	if (pdata->gpio_bridge_reset != -1) {
-		gpio_request(pdata->gpio_bridge_reset, "tc35876x bridge reset");
-		gpio_direction_output(pdata->gpio_bridge_reset, 0);
-	}
-
-	if (pdata->gpio_panel_bl_en != -1) {
-		gpio_request(pdata->gpio_panel_bl_en, "tc35876x panel bl en");
-		gpio_direction_output(pdata->gpio_panel_bl_en, 0);
-	}
+	bridge_bl_enable = devm_gpiod_get_optional(&client->dev, "bl-en", GPIOD_OUT_LOW);
+	if (IS_ERR(bridge_bl_enable))
+		return PTR_ERR(bridge_bl_enable);
+	if (bridge_bl_enable)
+		gpiod_set_consumer_name(bridge_bl_enable, "tc35876x panel bl en");
 
-	if (pdata->gpio_panel_vadd != -1) {
-		gpio_request(pdata->gpio_panel_vadd, "tc35876x panel vadd");
-		gpio_direction_output(pdata->gpio_panel_vadd, 0);
-	}
+	backlight_voltage = devm_gpiod_get_optional(&client->dev, "vadd", GPIOD_OUT_LOW);
+	if (IS_ERR(backlight_voltage))
+		return PTR_ERR(backlight_voltage);
+	if (backlight_voltage)
+		gpiod_set_consumer_name(backlight_voltage, "tc35876x panel vadd");
 
 	tc35876x_client = client;
 
@@ -673,19 +662,8 @@ static int tc35876x_bridge_probe(struct i2c_client *client,
 
 static int tc35876x_bridge_remove(struct i2c_client *client)
 {
-	struct tc35876x_platform_data *pdata = dev_get_platdata(&client->dev);
-
 	dev_dbg(&client->dev, "%s\n", __func__);
 
-	if (pdata->gpio_bridge_reset != -1)
-		gpio_free(pdata->gpio_bridge_reset);
-
-	if (pdata->gpio_panel_bl_en != -1)
-		gpio_free(pdata->gpio_panel_bl_en);
-
-	if (pdata->gpio_panel_vadd != -1)
-		gpio_free(pdata->gpio_panel_vadd);
-
 	tc35876x_client = NULL;
 
 	return 0;
diff --git a/include/linux/platform_data/tc35876x.h b/include/linux/platform_data/tc35876x.h
deleted file mode 100644
index cd6a51c71e7e..000000000000
--- a/include/linux/platform_data/tc35876x.h
+++ /dev/null
@@ -1,11 +0,0 @@
-
-#ifndef _TC35876X_H
-#define _TC35876X_H
-
-struct tc35876x_platform_data {
-	int gpio_bridge_reset;
-	int gpio_panel_bl_en;
-	int gpio_panel_vadd;
-};
-
-#endif /* _TC35876X_H */
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/gma500: Pass GPIO for Intel MID using descriptors
  2019-12-06  9:43 ` Linus Walleij
@ 2019-12-08 10:20   ` Andy Shevchenko
  -1 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2019-12-08 10:20 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Daniel Stone, Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
	Patrik Jakobsson, dri-devel, Sean Paul, linux-arm-kernel

On Fri, Dec 06, 2019 at 10:43:01AM +0100, Linus Walleij wrote:
> The GMA500 driver is using the legacy GPIO API to fetch
> three optional display control GPIO lines from the SFI
> description used by the Medfield platform.
> 
> Switch this over to use GPIO descriptors and delete the
> custom platform data.
> 
> We create three new static locals in the tc35876x bridge
> code but it is hardly any worse than the I2C client static
> local already there: I tried first to move it to the DRM
> driver state container but there are workarounds for
> probe order in the code so I just stayed off it, as the
> result is unpredictable.
> 
> People wanting to do a more throrugh and proper cleanup
> of the GMA500 driver can work on top of this, I can't
> solve much more since I don't have access to the hardware,
> I can only attempt to tidy up my GPIO corner.

What a nice clean up!
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> 
> Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> Cc: Daniel Stone <daniels@collabora.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> If someone can test this on the target platform I'd be
> happy.
> ---
>  .../intel-mid/device_libs/platform_tc35876x.c | 26 ++++--
>  drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c    | 88 +++++++------------
>  include/linux/platform_data/tc35876x.h        | 11 ---
>  3 files changed, 51 insertions(+), 74 deletions(-)
>  delete mode 100644 include/linux/platform_data/tc35876x.h
> 
> diff --git a/arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c b/arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c
> index 44d1f884c3d3..139738bbdd36 100644
> --- a/arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c
> +++ b/arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c
> @@ -6,21 +6,31 @@
>   * Author: Sathyanarayanan Kuppuswamy <sathyanarayanan.kuppuswamy@intel.com>
>   */
>  
> -#include <linux/gpio.h>
> -#include <linux/platform_data/tc35876x.h>
> +#include <linux/gpio/machine.h>
>  #include <asm/intel-mid.h>
>  
> +static struct gpiod_lookup_table tc35876x_gpio_table = {
> +	.dev_id	= "i2c_disp_brig",
> +	.table	= {
> +		GPIO_LOOKUP("0000:00:0c.0", -1, "bridge-reset", GPIO_ACTIVE_HIGH),
> +		GPIO_LOOKUP("0000:00:0c.0", -1, "bl-en", GPIO_ACTIVE_HIGH),
> +		GPIO_LOOKUP("0000:00:0c.0", -1, "vadd", GPIO_ACTIVE_HIGH),
> +		{ },
> +	},
> +};
> +
>  /*tc35876x DSI_LVDS bridge chip and panel platform data*/
>  static void *tc35876x_platform_data(void *data)
>  {
> -	static struct tc35876x_platform_data pdata;
> +	struct gpiod_lookup_table *table = &tc35876x_gpio_table;
> +	struct gpiod_lookup *lookup = table->table;
>  
> -	/* gpio pins set to -1 will not be used by the driver */
> -	pdata.gpio_bridge_reset = get_gpio_by_name("LCMB_RXEN");
> -	pdata.gpio_panel_bl_en = get_gpio_by_name("6S6P_BL_EN");
> -	pdata.gpio_panel_vadd = get_gpio_by_name("EN_VREG_LCD_V3P3");
> +	lookup[0].chip_hwnum = get_gpio_by_name("LCMB_RXEN");
> +	lookup[1].chip_hwnum = get_gpio_by_name("6S6P_BL_EN");
> +	lookup[2].chip_hwnum = get_gpio_by_name("EN_VREG_LCD_V3P3");
> +	gpiod_add_lookup_table(table);
>  
> -	return &pdata;
> +	return NULL;
>  }
>  
>  static const struct devs_id tc35876x_dev_id __initconst = {
> diff --git a/drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c b/drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c
> index 7de3ce637c7f..9e8224456ea2 100644
> --- a/drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c
> +++ b/drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c
> @@ -25,7 +25,7 @@
>  #include <linux/delay.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> -#include <linux/platform_data/tc35876x.h>
> +#include <linux/gpio/consumer.h>
>  
>  #include <asm/intel_scu_ipc.h>
>  
> @@ -36,6 +36,11 @@
>  
>  static struct i2c_client *tc35876x_client;
>  static struct i2c_client *cmi_lcd_i2c_client;
> +/* Panel GPIOs */
> +static struct gpio_desc *bridge_reset;
> +static struct gpio_desc *bridge_bl_enable;
> +static struct gpio_desc *backlight_voltage;
> +
>  
>  #define FLD_MASK(start, end)	(((1 << ((start) - (end) + 1)) - 1) << (end))
>  #define FLD_VAL(val, start, end) (((val) << (end)) & FLD_MASK(start, end))
> @@ -316,27 +321,23 @@ static int tc35876x_regr(struct i2c_client *client, u16 reg, u32 *value)
>  
>  void tc35876x_set_bridge_reset_state(struct drm_device *dev, int state)
>  {
> -	struct tc35876x_platform_data *pdata;
> -
>  	if (WARN(!tc35876x_client, "%s called before probe", __func__))
>  		return;
>  
>  	dev_dbg(&tc35876x_client->dev, "%s: state %d\n", __func__, state);
>  
> -	pdata = dev_get_platdata(&tc35876x_client->dev);
> -
> -	if (pdata->gpio_bridge_reset == -1)
> +	if (!bridge_reset)
>  		return;
>  
>  	if (state) {
> -		gpio_set_value_cansleep(pdata->gpio_bridge_reset, 0);
> +		gpiod_set_value_cansleep(bridge_reset, 0);
>  		mdelay(10);
>  	} else {
>  		/* Pull MIPI Bridge reset pin to Low */
> -		gpio_set_value_cansleep(pdata->gpio_bridge_reset, 0);
> +		gpiod_set_value_cansleep(bridge_reset, 0);
>  		mdelay(20);
>  		/* Pull MIPI Bridge reset pin to High */
> -		gpio_set_value_cansleep(pdata->gpio_bridge_reset, 1);
> +		gpiod_set_value_cansleep(bridge_reset, 1);
>  		mdelay(40);
>  	}
>  }
> @@ -510,25 +511,20 @@ void tc35876x_brightness_control(struct drm_device *dev, int level)
>  
>  void tc35876x_toshiba_bridge_panel_off(struct drm_device *dev)
>  {
> -	struct tc35876x_platform_data *pdata;
> -
>  	if (WARN(!tc35876x_client, "%s called before probe", __func__))
>  		return;
>  
>  	dev_dbg(&tc35876x_client->dev, "%s\n", __func__);
>  
> -	pdata = dev_get_platdata(&tc35876x_client->dev);
> -
> -	if (pdata->gpio_panel_bl_en != -1)
> -		gpio_set_value_cansleep(pdata->gpio_panel_bl_en, 0);
> +	if (bridge_bl_enable)
> +		gpiod_set_value_cansleep(bridge_bl_enable, 0);
>  
> -	if (pdata->gpio_panel_vadd != -1)
> -		gpio_set_value_cansleep(pdata->gpio_panel_vadd, 0);
> +	if (backlight_voltage)
> +		gpiod_set_value_cansleep(backlight_voltage, 0);
>  }
>  
>  void tc35876x_toshiba_bridge_panel_on(struct drm_device *dev)
>  {
> -	struct tc35876x_platform_data *pdata;
>  	struct drm_psb_private *dev_priv = dev->dev_private;
>  
>  	if (WARN(!tc35876x_client, "%s called before probe", __func__))
> @@ -536,10 +532,8 @@ void tc35876x_toshiba_bridge_panel_on(struct drm_device *dev)
>  
>  	dev_dbg(&tc35876x_client->dev, "%s\n", __func__);
>  
> -	pdata = dev_get_platdata(&tc35876x_client->dev);
> -
> -	if (pdata->gpio_panel_vadd != -1) {
> -		gpio_set_value_cansleep(pdata->gpio_panel_vadd, 1);
> +	if (backlight_voltage) {
> +		gpiod_set_value_cansleep(backlight_voltage, 1);
>  		msleep(260);
>  	}
>  
> @@ -571,8 +565,8 @@ void tc35876x_toshiba_bridge_panel_on(struct drm_device *dev)
>  				"i2c write failed (%d)\n", ret);
>  	}
>  
> -	if (pdata->gpio_panel_bl_en != -1)
> -		gpio_set_value_cansleep(pdata->gpio_panel_bl_en, 1);
> +	if (bridge_bl_enable)
> +		gpiod_set_value_cansleep(bridge_bl_enable, 1);
>  
>  	tc35876x_brightness_control(dev, dev_priv->brightness_adjusted);
>  }
> @@ -635,8 +629,6 @@ static int tc35876x_get_panel_info(struct drm_device *dev, int pipe,
>  static int tc35876x_bridge_probe(struct i2c_client *client,
>  				const struct i2c_device_id *id)
>  {
> -	struct tc35876x_platform_data *pdata;
> -
>  	dev_info(&client->dev, "%s\n", __func__);
>  
>  	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> @@ -645,26 +637,23 @@ static int tc35876x_bridge_probe(struct i2c_client *client,
>  		return -ENODEV;
>  	}
>  
> -	pdata = dev_get_platdata(&client->dev);
> -	if (!pdata) {
> -		dev_err(&client->dev, "%s: no platform data\n", __func__);
> -		return -ENODEV;
> -	}
> +	bridge_reset = devm_gpiod_get_optional(&client->dev, "bridge-reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(bridge_reset))
> +		return PTR_ERR(bridge_reset);
> +	if (bridge_reset)
> +		gpiod_set_consumer_name(bridge_reset, "tc35876x bridge reset");
>  
> -	if (pdata->gpio_bridge_reset != -1) {
> -		gpio_request(pdata->gpio_bridge_reset, "tc35876x bridge reset");
> -		gpio_direction_output(pdata->gpio_bridge_reset, 0);
> -	}
> -
> -	if (pdata->gpio_panel_bl_en != -1) {
> -		gpio_request(pdata->gpio_panel_bl_en, "tc35876x panel bl en");
> -		gpio_direction_output(pdata->gpio_panel_bl_en, 0);
> -	}
> +	bridge_bl_enable = devm_gpiod_get_optional(&client->dev, "bl-en", GPIOD_OUT_LOW);
> +	if (IS_ERR(bridge_bl_enable))
> +		return PTR_ERR(bridge_bl_enable);
> +	if (bridge_bl_enable)
> +		gpiod_set_consumer_name(bridge_bl_enable, "tc35876x panel bl en");
>  
> -	if (pdata->gpio_panel_vadd != -1) {
> -		gpio_request(pdata->gpio_panel_vadd, "tc35876x panel vadd");
> -		gpio_direction_output(pdata->gpio_panel_vadd, 0);
> -	}
> +	backlight_voltage = devm_gpiod_get_optional(&client->dev, "vadd", GPIOD_OUT_LOW);
> +	if (IS_ERR(backlight_voltage))
> +		return PTR_ERR(backlight_voltage);
> +	if (backlight_voltage)
> +		gpiod_set_consumer_name(backlight_voltage, "tc35876x panel vadd");
>  
>  	tc35876x_client = client;
>  
> @@ -673,19 +662,8 @@ static int tc35876x_bridge_probe(struct i2c_client *client,
>  
>  static int tc35876x_bridge_remove(struct i2c_client *client)
>  {
> -	struct tc35876x_platform_data *pdata = dev_get_platdata(&client->dev);
> -
>  	dev_dbg(&client->dev, "%s\n", __func__);
>  
> -	if (pdata->gpio_bridge_reset != -1)
> -		gpio_free(pdata->gpio_bridge_reset);
> -
> -	if (pdata->gpio_panel_bl_en != -1)
> -		gpio_free(pdata->gpio_panel_bl_en);
> -
> -	if (pdata->gpio_panel_vadd != -1)
> -		gpio_free(pdata->gpio_panel_vadd);
> -
>  	tc35876x_client = NULL;
>  
>  	return 0;
> diff --git a/include/linux/platform_data/tc35876x.h b/include/linux/platform_data/tc35876x.h
> deleted file mode 100644
> index cd6a51c71e7e..000000000000
> --- a/include/linux/platform_data/tc35876x.h
> +++ /dev/null
> @@ -1,11 +0,0 @@
> -
> -#ifndef _TC35876X_H
> -#define _TC35876X_H
> -
> -struct tc35876x_platform_data {
> -	int gpio_bridge_reset;
> -	int gpio_panel_bl_en;
> -	int gpio_panel_vadd;
> -};
> -
> -#endif /* _TC35876X_H */
> -- 
> 2.23.0
> 

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/gma500: Pass GPIO for Intel MID using descriptors
@ 2019-12-08 10:20   ` Andy Shevchenko
  0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2019-12-08 10:20 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Daniel Stone, Daniel Vetter, dri-devel, Sean Paul, linux-arm-kernel

On Fri, Dec 06, 2019 at 10:43:01AM +0100, Linus Walleij wrote:
> The GMA500 driver is using the legacy GPIO API to fetch
> three optional display control GPIO lines from the SFI
> description used by the Medfield platform.
> 
> Switch this over to use GPIO descriptors and delete the
> custom platform data.
> 
> We create three new static locals in the tc35876x bridge
> code but it is hardly any worse than the I2C client static
> local already there: I tried first to move it to the DRM
> driver state container but there are workarounds for
> probe order in the code so I just stayed off it, as the
> result is unpredictable.
> 
> People wanting to do a more throrugh and proper cleanup
> of the GMA500 driver can work on top of this, I can't
> solve much more since I don't have access to the hardware,
> I can only attempt to tidy up my GPIO corner.

What a nice clean up!
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> 
> Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> Cc: Daniel Stone <daniels@collabora.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> If someone can test this on the target platform I'd be
> happy.
> ---
>  .../intel-mid/device_libs/platform_tc35876x.c | 26 ++++--
>  drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c    | 88 +++++++------------
>  include/linux/platform_data/tc35876x.h        | 11 ---
>  3 files changed, 51 insertions(+), 74 deletions(-)
>  delete mode 100644 include/linux/platform_data/tc35876x.h
> 
> diff --git a/arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c b/arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c
> index 44d1f884c3d3..139738bbdd36 100644
> --- a/arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c
> +++ b/arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c
> @@ -6,21 +6,31 @@
>   * Author: Sathyanarayanan Kuppuswamy <sathyanarayanan.kuppuswamy@intel.com>
>   */
>  
> -#include <linux/gpio.h>
> -#include <linux/platform_data/tc35876x.h>
> +#include <linux/gpio/machine.h>
>  #include <asm/intel-mid.h>
>  
> +static struct gpiod_lookup_table tc35876x_gpio_table = {
> +	.dev_id	= "i2c_disp_brig",
> +	.table	= {
> +		GPIO_LOOKUP("0000:00:0c.0", -1, "bridge-reset", GPIO_ACTIVE_HIGH),
> +		GPIO_LOOKUP("0000:00:0c.0", -1, "bl-en", GPIO_ACTIVE_HIGH),
> +		GPIO_LOOKUP("0000:00:0c.0", -1, "vadd", GPIO_ACTIVE_HIGH),
> +		{ },
> +	},
> +};
> +
>  /*tc35876x DSI_LVDS bridge chip and panel platform data*/
>  static void *tc35876x_platform_data(void *data)
>  {
> -	static struct tc35876x_platform_data pdata;
> +	struct gpiod_lookup_table *table = &tc35876x_gpio_table;
> +	struct gpiod_lookup *lookup = table->table;
>  
> -	/* gpio pins set to -1 will not be used by the driver */
> -	pdata.gpio_bridge_reset = get_gpio_by_name("LCMB_RXEN");
> -	pdata.gpio_panel_bl_en = get_gpio_by_name("6S6P_BL_EN");
> -	pdata.gpio_panel_vadd = get_gpio_by_name("EN_VREG_LCD_V3P3");
> +	lookup[0].chip_hwnum = get_gpio_by_name("LCMB_RXEN");
> +	lookup[1].chip_hwnum = get_gpio_by_name("6S6P_BL_EN");
> +	lookup[2].chip_hwnum = get_gpio_by_name("EN_VREG_LCD_V3P3");
> +	gpiod_add_lookup_table(table);
>  
> -	return &pdata;
> +	return NULL;
>  }
>  
>  static const struct devs_id tc35876x_dev_id __initconst = {
> diff --git a/drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c b/drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c
> index 7de3ce637c7f..9e8224456ea2 100644
> --- a/drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c
> +++ b/drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c
> @@ -25,7 +25,7 @@
>  #include <linux/delay.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> -#include <linux/platform_data/tc35876x.h>
> +#include <linux/gpio/consumer.h>
>  
>  #include <asm/intel_scu_ipc.h>
>  
> @@ -36,6 +36,11 @@
>  
>  static struct i2c_client *tc35876x_client;
>  static struct i2c_client *cmi_lcd_i2c_client;
> +/* Panel GPIOs */
> +static struct gpio_desc *bridge_reset;
> +static struct gpio_desc *bridge_bl_enable;
> +static struct gpio_desc *backlight_voltage;
> +
>  
>  #define FLD_MASK(start, end)	(((1 << ((start) - (end) + 1)) - 1) << (end))
>  #define FLD_VAL(val, start, end) (((val) << (end)) & FLD_MASK(start, end))
> @@ -316,27 +321,23 @@ static int tc35876x_regr(struct i2c_client *client, u16 reg, u32 *value)
>  
>  void tc35876x_set_bridge_reset_state(struct drm_device *dev, int state)
>  {
> -	struct tc35876x_platform_data *pdata;
> -
>  	if (WARN(!tc35876x_client, "%s called before probe", __func__))
>  		return;
>  
>  	dev_dbg(&tc35876x_client->dev, "%s: state %d\n", __func__, state);
>  
> -	pdata = dev_get_platdata(&tc35876x_client->dev);
> -
> -	if (pdata->gpio_bridge_reset == -1)
> +	if (!bridge_reset)
>  		return;
>  
>  	if (state) {
> -		gpio_set_value_cansleep(pdata->gpio_bridge_reset, 0);
> +		gpiod_set_value_cansleep(bridge_reset, 0);
>  		mdelay(10);
>  	} else {
>  		/* Pull MIPI Bridge reset pin to Low */
> -		gpio_set_value_cansleep(pdata->gpio_bridge_reset, 0);
> +		gpiod_set_value_cansleep(bridge_reset, 0);
>  		mdelay(20);
>  		/* Pull MIPI Bridge reset pin to High */
> -		gpio_set_value_cansleep(pdata->gpio_bridge_reset, 1);
> +		gpiod_set_value_cansleep(bridge_reset, 1);
>  		mdelay(40);
>  	}
>  }
> @@ -510,25 +511,20 @@ void tc35876x_brightness_control(struct drm_device *dev, int level)
>  
>  void tc35876x_toshiba_bridge_panel_off(struct drm_device *dev)
>  {
> -	struct tc35876x_platform_data *pdata;
> -
>  	if (WARN(!tc35876x_client, "%s called before probe", __func__))
>  		return;
>  
>  	dev_dbg(&tc35876x_client->dev, "%s\n", __func__);
>  
> -	pdata = dev_get_platdata(&tc35876x_client->dev);
> -
> -	if (pdata->gpio_panel_bl_en != -1)
> -		gpio_set_value_cansleep(pdata->gpio_panel_bl_en, 0);
> +	if (bridge_bl_enable)
> +		gpiod_set_value_cansleep(bridge_bl_enable, 0);
>  
> -	if (pdata->gpio_panel_vadd != -1)
> -		gpio_set_value_cansleep(pdata->gpio_panel_vadd, 0);
> +	if (backlight_voltage)
> +		gpiod_set_value_cansleep(backlight_voltage, 0);
>  }
>  
>  void tc35876x_toshiba_bridge_panel_on(struct drm_device *dev)
>  {
> -	struct tc35876x_platform_data *pdata;
>  	struct drm_psb_private *dev_priv = dev->dev_private;
>  
>  	if (WARN(!tc35876x_client, "%s called before probe", __func__))
> @@ -536,10 +532,8 @@ void tc35876x_toshiba_bridge_panel_on(struct drm_device *dev)
>  
>  	dev_dbg(&tc35876x_client->dev, "%s\n", __func__);
>  
> -	pdata = dev_get_platdata(&tc35876x_client->dev);
> -
> -	if (pdata->gpio_panel_vadd != -1) {
> -		gpio_set_value_cansleep(pdata->gpio_panel_vadd, 1);
> +	if (backlight_voltage) {
> +		gpiod_set_value_cansleep(backlight_voltage, 1);
>  		msleep(260);
>  	}
>  
> @@ -571,8 +565,8 @@ void tc35876x_toshiba_bridge_panel_on(struct drm_device *dev)
>  				"i2c write failed (%d)\n", ret);
>  	}
>  
> -	if (pdata->gpio_panel_bl_en != -1)
> -		gpio_set_value_cansleep(pdata->gpio_panel_bl_en, 1);
> +	if (bridge_bl_enable)
> +		gpiod_set_value_cansleep(bridge_bl_enable, 1);
>  
>  	tc35876x_brightness_control(dev, dev_priv->brightness_adjusted);
>  }
> @@ -635,8 +629,6 @@ static int tc35876x_get_panel_info(struct drm_device *dev, int pipe,
>  static int tc35876x_bridge_probe(struct i2c_client *client,
>  				const struct i2c_device_id *id)
>  {
> -	struct tc35876x_platform_data *pdata;
> -
>  	dev_info(&client->dev, "%s\n", __func__);
>  
>  	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> @@ -645,26 +637,23 @@ static int tc35876x_bridge_probe(struct i2c_client *client,
>  		return -ENODEV;
>  	}
>  
> -	pdata = dev_get_platdata(&client->dev);
> -	if (!pdata) {
> -		dev_err(&client->dev, "%s: no platform data\n", __func__);
> -		return -ENODEV;
> -	}
> +	bridge_reset = devm_gpiod_get_optional(&client->dev, "bridge-reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(bridge_reset))
> +		return PTR_ERR(bridge_reset);
> +	if (bridge_reset)
> +		gpiod_set_consumer_name(bridge_reset, "tc35876x bridge reset");
>  
> -	if (pdata->gpio_bridge_reset != -1) {
> -		gpio_request(pdata->gpio_bridge_reset, "tc35876x bridge reset");
> -		gpio_direction_output(pdata->gpio_bridge_reset, 0);
> -	}
> -
> -	if (pdata->gpio_panel_bl_en != -1) {
> -		gpio_request(pdata->gpio_panel_bl_en, "tc35876x panel bl en");
> -		gpio_direction_output(pdata->gpio_panel_bl_en, 0);
> -	}
> +	bridge_bl_enable = devm_gpiod_get_optional(&client->dev, "bl-en", GPIOD_OUT_LOW);
> +	if (IS_ERR(bridge_bl_enable))
> +		return PTR_ERR(bridge_bl_enable);
> +	if (bridge_bl_enable)
> +		gpiod_set_consumer_name(bridge_bl_enable, "tc35876x panel bl en");
>  
> -	if (pdata->gpio_panel_vadd != -1) {
> -		gpio_request(pdata->gpio_panel_vadd, "tc35876x panel vadd");
> -		gpio_direction_output(pdata->gpio_panel_vadd, 0);
> -	}
> +	backlight_voltage = devm_gpiod_get_optional(&client->dev, "vadd", GPIOD_OUT_LOW);
> +	if (IS_ERR(backlight_voltage))
> +		return PTR_ERR(backlight_voltage);
> +	if (backlight_voltage)
> +		gpiod_set_consumer_name(backlight_voltage, "tc35876x panel vadd");
>  
>  	tc35876x_client = client;
>  
> @@ -673,19 +662,8 @@ static int tc35876x_bridge_probe(struct i2c_client *client,
>  
>  static int tc35876x_bridge_remove(struct i2c_client *client)
>  {
> -	struct tc35876x_platform_data *pdata = dev_get_platdata(&client->dev);
> -
>  	dev_dbg(&client->dev, "%s\n", __func__);
>  
> -	if (pdata->gpio_bridge_reset != -1)
> -		gpio_free(pdata->gpio_bridge_reset);
> -
> -	if (pdata->gpio_panel_bl_en != -1)
> -		gpio_free(pdata->gpio_panel_bl_en);
> -
> -	if (pdata->gpio_panel_vadd != -1)
> -		gpio_free(pdata->gpio_panel_vadd);
> -
>  	tc35876x_client = NULL;
>  
>  	return 0;
> diff --git a/include/linux/platform_data/tc35876x.h b/include/linux/platform_data/tc35876x.h
> deleted file mode 100644
> index cd6a51c71e7e..000000000000
> --- a/include/linux/platform_data/tc35876x.h
> +++ /dev/null
> @@ -1,11 +0,0 @@
> -
> -#ifndef _TC35876X_H
> -#define _TC35876X_H
> -
> -struct tc35876x_platform_data {
> -	int gpio_bridge_reset;
> -	int gpio_panel_bl_en;
> -	int gpio_panel_vadd;
> -};
> -
> -#endif /* _TC35876X_H */
> -- 
> 2.23.0
> 

-- 
With Best Regards,
Andy Shevchenko


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/gma500: Pass GPIO for Intel MID using descriptors
  2019-12-06  9:43 ` Linus Walleij
@ 2019-12-09 10:32   ` Patrik Jakobsson
  -1 siblings, 0 replies; 6+ messages in thread
From: Patrik Jakobsson @ 2019-12-09 10:32 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Daniel Stone, Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
	dri-devel, Andy Shevchenko, Sean Paul, linux-arm-kernel

On Fri, Dec 6, 2019 at 10:43 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> The GMA500 driver is using the legacy GPIO API to fetch
> three optional display control GPIO lines from the SFI
> description used by the Medfield platform.
>
> Switch this over to use GPIO descriptors and delete the
> custom platform data.
>
> We create three new static locals in the tc35876x bridge
> code but it is hardly any worse than the I2C client static
> local already there: I tried first to move it to the DRM
> driver state container but there are workarounds for
> probe order in the code so I just stayed off it, as the
> result is unpredictable.
>
> People wanting to do a more throrugh and proper cleanup
> of the GMA500 driver can work on top of this, I can't
> solve much more since I don't have access to the hardware,
> I can only attempt to tidy up my GPIO corner.
>
> Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> Cc: Daniel Stone <daniels@collabora.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> If someone can test this on the target platform I'd be
> happy.

Hi Linus, sorry don't have hardware to test with. Medfields are very
rare (or extinct).

This looks ok. I'll let you apply this. Thanks for the cleanup.

Acked-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>

> ---
>  .../intel-mid/device_libs/platform_tc35876x.c | 26 ++++--
>  drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c    | 88 +++++++------------
>  include/linux/platform_data/tc35876x.h        | 11 ---
>  3 files changed, 51 insertions(+), 74 deletions(-)
>  delete mode 100644 include/linux/platform_data/tc35876x.h
>
> diff --git a/arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c b/arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c
> index 44d1f884c3d3..139738bbdd36 100644
> --- a/arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c
> +++ b/arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c
> @@ -6,21 +6,31 @@
>   * Author: Sathyanarayanan Kuppuswamy <sathyanarayanan.kuppuswamy@intel.com>
>   */
>
> -#include <linux/gpio.h>
> -#include <linux/platform_data/tc35876x.h>
> +#include <linux/gpio/machine.h>
>  #include <asm/intel-mid.h>
>
> +static struct gpiod_lookup_table tc35876x_gpio_table = {
> +       .dev_id = "i2c_disp_brig",
> +       .table  = {
> +               GPIO_LOOKUP("0000:00:0c.0", -1, "bridge-reset", GPIO_ACTIVE_HIGH),
> +               GPIO_LOOKUP("0000:00:0c.0", -1, "bl-en", GPIO_ACTIVE_HIGH),
> +               GPIO_LOOKUP("0000:00:0c.0", -1, "vadd", GPIO_ACTIVE_HIGH),
> +               { },
> +       },
> +};
> +
>  /*tc35876x DSI_LVDS bridge chip and panel platform data*/
>  static void *tc35876x_platform_data(void *data)
>  {
> -       static struct tc35876x_platform_data pdata;
> +       struct gpiod_lookup_table *table = &tc35876x_gpio_table;
> +       struct gpiod_lookup *lookup = table->table;
>
> -       /* gpio pins set to -1 will not be used by the driver */
> -       pdata.gpio_bridge_reset = get_gpio_by_name("LCMB_RXEN");
> -       pdata.gpio_panel_bl_en = get_gpio_by_name("6S6P_BL_EN");
> -       pdata.gpio_panel_vadd = get_gpio_by_name("EN_VREG_LCD_V3P3");
> +       lookup[0].chip_hwnum = get_gpio_by_name("LCMB_RXEN");
> +       lookup[1].chip_hwnum = get_gpio_by_name("6S6P_BL_EN");
> +       lookup[2].chip_hwnum = get_gpio_by_name("EN_VREG_LCD_V3P3");
> +       gpiod_add_lookup_table(table);
>
> -       return &pdata;
> +       return NULL;
>  }
>
>  static const struct devs_id tc35876x_dev_id __initconst = {
> diff --git a/drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c b/drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c
> index 7de3ce637c7f..9e8224456ea2 100644
> --- a/drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c
> +++ b/drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c
> @@ -25,7 +25,7 @@
>  #include <linux/delay.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> -#include <linux/platform_data/tc35876x.h>
> +#include <linux/gpio/consumer.h>
>
>  #include <asm/intel_scu_ipc.h>
>
> @@ -36,6 +36,11 @@
>
>  static struct i2c_client *tc35876x_client;
>  static struct i2c_client *cmi_lcd_i2c_client;
> +/* Panel GPIOs */
> +static struct gpio_desc *bridge_reset;
> +static struct gpio_desc *bridge_bl_enable;
> +static struct gpio_desc *backlight_voltage;
> +
>
>  #define FLD_MASK(start, end)   (((1 << ((start) - (end) + 1)) - 1) << (end))
>  #define FLD_VAL(val, start, end) (((val) << (end)) & FLD_MASK(start, end))
> @@ -316,27 +321,23 @@ static int tc35876x_regr(struct i2c_client *client, u16 reg, u32 *value)
>
>  void tc35876x_set_bridge_reset_state(struct drm_device *dev, int state)
>  {
> -       struct tc35876x_platform_data *pdata;
> -
>         if (WARN(!tc35876x_client, "%s called before probe", __func__))
>                 return;
>
>         dev_dbg(&tc35876x_client->dev, "%s: state %d\n", __func__, state);
>
> -       pdata = dev_get_platdata(&tc35876x_client->dev);
> -
> -       if (pdata->gpio_bridge_reset == -1)
> +       if (!bridge_reset)
>                 return;
>
>         if (state) {
> -               gpio_set_value_cansleep(pdata->gpio_bridge_reset, 0);
> +               gpiod_set_value_cansleep(bridge_reset, 0);
>                 mdelay(10);
>         } else {
>                 /* Pull MIPI Bridge reset pin to Low */
> -               gpio_set_value_cansleep(pdata->gpio_bridge_reset, 0);
> +               gpiod_set_value_cansleep(bridge_reset, 0);
>                 mdelay(20);
>                 /* Pull MIPI Bridge reset pin to High */
> -               gpio_set_value_cansleep(pdata->gpio_bridge_reset, 1);
> +               gpiod_set_value_cansleep(bridge_reset, 1);
>                 mdelay(40);
>         }
>  }
> @@ -510,25 +511,20 @@ void tc35876x_brightness_control(struct drm_device *dev, int level)
>
>  void tc35876x_toshiba_bridge_panel_off(struct drm_device *dev)
>  {
> -       struct tc35876x_platform_data *pdata;
> -
>         if (WARN(!tc35876x_client, "%s called before probe", __func__))
>                 return;
>
>         dev_dbg(&tc35876x_client->dev, "%s\n", __func__);
>
> -       pdata = dev_get_platdata(&tc35876x_client->dev);
> -
> -       if (pdata->gpio_panel_bl_en != -1)
> -               gpio_set_value_cansleep(pdata->gpio_panel_bl_en, 0);
> +       if (bridge_bl_enable)
> +               gpiod_set_value_cansleep(bridge_bl_enable, 0);
>
> -       if (pdata->gpio_panel_vadd != -1)
> -               gpio_set_value_cansleep(pdata->gpio_panel_vadd, 0);
> +       if (backlight_voltage)
> +               gpiod_set_value_cansleep(backlight_voltage, 0);
>  }
>
>  void tc35876x_toshiba_bridge_panel_on(struct drm_device *dev)
>  {
> -       struct tc35876x_platform_data *pdata;
>         struct drm_psb_private *dev_priv = dev->dev_private;
>
>         if (WARN(!tc35876x_client, "%s called before probe", __func__))
> @@ -536,10 +532,8 @@ void tc35876x_toshiba_bridge_panel_on(struct drm_device *dev)
>
>         dev_dbg(&tc35876x_client->dev, "%s\n", __func__);
>
> -       pdata = dev_get_platdata(&tc35876x_client->dev);
> -
> -       if (pdata->gpio_panel_vadd != -1) {
> -               gpio_set_value_cansleep(pdata->gpio_panel_vadd, 1);
> +       if (backlight_voltage) {
> +               gpiod_set_value_cansleep(backlight_voltage, 1);
>                 msleep(260);
>         }
>
> @@ -571,8 +565,8 @@ void tc35876x_toshiba_bridge_panel_on(struct drm_device *dev)
>                                 "i2c write failed (%d)\n", ret);
>         }
>
> -       if (pdata->gpio_panel_bl_en != -1)
> -               gpio_set_value_cansleep(pdata->gpio_panel_bl_en, 1);
> +       if (bridge_bl_enable)
> +               gpiod_set_value_cansleep(bridge_bl_enable, 1);
>
>         tc35876x_brightness_control(dev, dev_priv->brightness_adjusted);
>  }
> @@ -635,8 +629,6 @@ static int tc35876x_get_panel_info(struct drm_device *dev, int pipe,
>  static int tc35876x_bridge_probe(struct i2c_client *client,
>                                 const struct i2c_device_id *id)
>  {
> -       struct tc35876x_platform_data *pdata;
> -
>         dev_info(&client->dev, "%s\n", __func__);
>
>         if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> @@ -645,26 +637,23 @@ static int tc35876x_bridge_probe(struct i2c_client *client,
>                 return -ENODEV;
>         }
>
> -       pdata = dev_get_platdata(&client->dev);
> -       if (!pdata) {
> -               dev_err(&client->dev, "%s: no platform data\n", __func__);
> -               return -ENODEV;
> -       }
> +       bridge_reset = devm_gpiod_get_optional(&client->dev, "bridge-reset", GPIOD_OUT_LOW);
> +       if (IS_ERR(bridge_reset))
> +               return PTR_ERR(bridge_reset);
> +       if (bridge_reset)
> +               gpiod_set_consumer_name(bridge_reset, "tc35876x bridge reset");
>
> -       if (pdata->gpio_bridge_reset != -1) {
> -               gpio_request(pdata->gpio_bridge_reset, "tc35876x bridge reset");
> -               gpio_direction_output(pdata->gpio_bridge_reset, 0);
> -       }
> -
> -       if (pdata->gpio_panel_bl_en != -1) {
> -               gpio_request(pdata->gpio_panel_bl_en, "tc35876x panel bl en");
> -               gpio_direction_output(pdata->gpio_panel_bl_en, 0);
> -       }
> +       bridge_bl_enable = devm_gpiod_get_optional(&client->dev, "bl-en", GPIOD_OUT_LOW);
> +       if (IS_ERR(bridge_bl_enable))
> +               return PTR_ERR(bridge_bl_enable);
> +       if (bridge_bl_enable)
> +               gpiod_set_consumer_name(bridge_bl_enable, "tc35876x panel bl en");
>
> -       if (pdata->gpio_panel_vadd != -1) {
> -               gpio_request(pdata->gpio_panel_vadd, "tc35876x panel vadd");
> -               gpio_direction_output(pdata->gpio_panel_vadd, 0);
> -       }
> +       backlight_voltage = devm_gpiod_get_optional(&client->dev, "vadd", GPIOD_OUT_LOW);
> +       if (IS_ERR(backlight_voltage))
> +               return PTR_ERR(backlight_voltage);
> +       if (backlight_voltage)
> +               gpiod_set_consumer_name(backlight_voltage, "tc35876x panel vadd");
>
>         tc35876x_client = client;
>
> @@ -673,19 +662,8 @@ static int tc35876x_bridge_probe(struct i2c_client *client,
>
>  static int tc35876x_bridge_remove(struct i2c_client *client)
>  {
> -       struct tc35876x_platform_data *pdata = dev_get_platdata(&client->dev);
> -
>         dev_dbg(&client->dev, "%s\n", __func__);
>
> -       if (pdata->gpio_bridge_reset != -1)
> -               gpio_free(pdata->gpio_bridge_reset);
> -
> -       if (pdata->gpio_panel_bl_en != -1)
> -               gpio_free(pdata->gpio_panel_bl_en);
> -
> -       if (pdata->gpio_panel_vadd != -1)
> -               gpio_free(pdata->gpio_panel_vadd);
> -
>         tc35876x_client = NULL;
>
>         return 0;
> diff --git a/include/linux/platform_data/tc35876x.h b/include/linux/platform_data/tc35876x.h
> deleted file mode 100644
> index cd6a51c71e7e..000000000000
> --- a/include/linux/platform_data/tc35876x.h
> +++ /dev/null
> @@ -1,11 +0,0 @@
> -
> -#ifndef _TC35876X_H
> -#define _TC35876X_H
> -
> -struct tc35876x_platform_data {
> -       int gpio_bridge_reset;
> -       int gpio_panel_bl_en;
> -       int gpio_panel_vadd;
> -};
> -
> -#endif /* _TC35876X_H */
> --
> 2.23.0
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/gma500: Pass GPIO for Intel MID using descriptors
@ 2019-12-09 10:32   ` Patrik Jakobsson
  0 siblings, 0 replies; 6+ messages in thread
From: Patrik Jakobsson @ 2019-12-09 10:32 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Daniel Stone, Daniel Vetter, dri-devel, Andy Shevchenko,
	Sean Paul, linux-arm-kernel

On Fri, Dec 6, 2019 at 10:43 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> The GMA500 driver is using the legacy GPIO API to fetch
> three optional display control GPIO lines from the SFI
> description used by the Medfield platform.
>
> Switch this over to use GPIO descriptors and delete the
> custom platform data.
>
> We create three new static locals in the tc35876x bridge
> code but it is hardly any worse than the I2C client static
> local already there: I tried first to move it to the DRM
> driver state container but there are workarounds for
> probe order in the code so I just stayed off it, as the
> result is unpredictable.
>
> People wanting to do a more throrugh and proper cleanup
> of the GMA500 driver can work on top of this, I can't
> solve much more since I don't have access to the hardware,
> I can only attempt to tidy up my GPIO corner.
>
> Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> Cc: Daniel Stone <daniels@collabora.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> If someone can test this on the target platform I'd be
> happy.

Hi Linus, sorry don't have hardware to test with. Medfields are very
rare (or extinct).

This looks ok. I'll let you apply this. Thanks for the cleanup.

Acked-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>

> ---
>  .../intel-mid/device_libs/platform_tc35876x.c | 26 ++++--
>  drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c    | 88 +++++++------------
>  include/linux/platform_data/tc35876x.h        | 11 ---
>  3 files changed, 51 insertions(+), 74 deletions(-)
>  delete mode 100644 include/linux/platform_data/tc35876x.h
>
> diff --git a/arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c b/arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c
> index 44d1f884c3d3..139738bbdd36 100644
> --- a/arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c
> +++ b/arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c
> @@ -6,21 +6,31 @@
>   * Author: Sathyanarayanan Kuppuswamy <sathyanarayanan.kuppuswamy@intel.com>
>   */
>
> -#include <linux/gpio.h>
> -#include <linux/platform_data/tc35876x.h>
> +#include <linux/gpio/machine.h>
>  #include <asm/intel-mid.h>
>
> +static struct gpiod_lookup_table tc35876x_gpio_table = {
> +       .dev_id = "i2c_disp_brig",
> +       .table  = {
> +               GPIO_LOOKUP("0000:00:0c.0", -1, "bridge-reset", GPIO_ACTIVE_HIGH),
> +               GPIO_LOOKUP("0000:00:0c.0", -1, "bl-en", GPIO_ACTIVE_HIGH),
> +               GPIO_LOOKUP("0000:00:0c.0", -1, "vadd", GPIO_ACTIVE_HIGH),
> +               { },
> +       },
> +};
> +
>  /*tc35876x DSI_LVDS bridge chip and panel platform data*/
>  static void *tc35876x_platform_data(void *data)
>  {
> -       static struct tc35876x_platform_data pdata;
> +       struct gpiod_lookup_table *table = &tc35876x_gpio_table;
> +       struct gpiod_lookup *lookup = table->table;
>
> -       /* gpio pins set to -1 will not be used by the driver */
> -       pdata.gpio_bridge_reset = get_gpio_by_name("LCMB_RXEN");
> -       pdata.gpio_panel_bl_en = get_gpio_by_name("6S6P_BL_EN");
> -       pdata.gpio_panel_vadd = get_gpio_by_name("EN_VREG_LCD_V3P3");
> +       lookup[0].chip_hwnum = get_gpio_by_name("LCMB_RXEN");
> +       lookup[1].chip_hwnum = get_gpio_by_name("6S6P_BL_EN");
> +       lookup[2].chip_hwnum = get_gpio_by_name("EN_VREG_LCD_V3P3");
> +       gpiod_add_lookup_table(table);
>
> -       return &pdata;
> +       return NULL;
>  }
>
>  static const struct devs_id tc35876x_dev_id __initconst = {
> diff --git a/drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c b/drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c
> index 7de3ce637c7f..9e8224456ea2 100644
> --- a/drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c
> +++ b/drivers/gpu/drm/gma500/tc35876x-dsi-lvds.c
> @@ -25,7 +25,7 @@
>  #include <linux/delay.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> -#include <linux/platform_data/tc35876x.h>
> +#include <linux/gpio/consumer.h>
>
>  #include <asm/intel_scu_ipc.h>
>
> @@ -36,6 +36,11 @@
>
>  static struct i2c_client *tc35876x_client;
>  static struct i2c_client *cmi_lcd_i2c_client;
> +/* Panel GPIOs */
> +static struct gpio_desc *bridge_reset;
> +static struct gpio_desc *bridge_bl_enable;
> +static struct gpio_desc *backlight_voltage;
> +
>
>  #define FLD_MASK(start, end)   (((1 << ((start) - (end) + 1)) - 1) << (end))
>  #define FLD_VAL(val, start, end) (((val) << (end)) & FLD_MASK(start, end))
> @@ -316,27 +321,23 @@ static int tc35876x_regr(struct i2c_client *client, u16 reg, u32 *value)
>
>  void tc35876x_set_bridge_reset_state(struct drm_device *dev, int state)
>  {
> -       struct tc35876x_platform_data *pdata;
> -
>         if (WARN(!tc35876x_client, "%s called before probe", __func__))
>                 return;
>
>         dev_dbg(&tc35876x_client->dev, "%s: state %d\n", __func__, state);
>
> -       pdata = dev_get_platdata(&tc35876x_client->dev);
> -
> -       if (pdata->gpio_bridge_reset == -1)
> +       if (!bridge_reset)
>                 return;
>
>         if (state) {
> -               gpio_set_value_cansleep(pdata->gpio_bridge_reset, 0);
> +               gpiod_set_value_cansleep(bridge_reset, 0);
>                 mdelay(10);
>         } else {
>                 /* Pull MIPI Bridge reset pin to Low */
> -               gpio_set_value_cansleep(pdata->gpio_bridge_reset, 0);
> +               gpiod_set_value_cansleep(bridge_reset, 0);
>                 mdelay(20);
>                 /* Pull MIPI Bridge reset pin to High */
> -               gpio_set_value_cansleep(pdata->gpio_bridge_reset, 1);
> +               gpiod_set_value_cansleep(bridge_reset, 1);
>                 mdelay(40);
>         }
>  }
> @@ -510,25 +511,20 @@ void tc35876x_brightness_control(struct drm_device *dev, int level)
>
>  void tc35876x_toshiba_bridge_panel_off(struct drm_device *dev)
>  {
> -       struct tc35876x_platform_data *pdata;
> -
>         if (WARN(!tc35876x_client, "%s called before probe", __func__))
>                 return;
>
>         dev_dbg(&tc35876x_client->dev, "%s\n", __func__);
>
> -       pdata = dev_get_platdata(&tc35876x_client->dev);
> -
> -       if (pdata->gpio_panel_bl_en != -1)
> -               gpio_set_value_cansleep(pdata->gpio_panel_bl_en, 0);
> +       if (bridge_bl_enable)
> +               gpiod_set_value_cansleep(bridge_bl_enable, 0);
>
> -       if (pdata->gpio_panel_vadd != -1)
> -               gpio_set_value_cansleep(pdata->gpio_panel_vadd, 0);
> +       if (backlight_voltage)
> +               gpiod_set_value_cansleep(backlight_voltage, 0);
>  }
>
>  void tc35876x_toshiba_bridge_panel_on(struct drm_device *dev)
>  {
> -       struct tc35876x_platform_data *pdata;
>         struct drm_psb_private *dev_priv = dev->dev_private;
>
>         if (WARN(!tc35876x_client, "%s called before probe", __func__))
> @@ -536,10 +532,8 @@ void tc35876x_toshiba_bridge_panel_on(struct drm_device *dev)
>
>         dev_dbg(&tc35876x_client->dev, "%s\n", __func__);
>
> -       pdata = dev_get_platdata(&tc35876x_client->dev);
> -
> -       if (pdata->gpio_panel_vadd != -1) {
> -               gpio_set_value_cansleep(pdata->gpio_panel_vadd, 1);
> +       if (backlight_voltage) {
> +               gpiod_set_value_cansleep(backlight_voltage, 1);
>                 msleep(260);
>         }
>
> @@ -571,8 +565,8 @@ void tc35876x_toshiba_bridge_panel_on(struct drm_device *dev)
>                                 "i2c write failed (%d)\n", ret);
>         }
>
> -       if (pdata->gpio_panel_bl_en != -1)
> -               gpio_set_value_cansleep(pdata->gpio_panel_bl_en, 1);
> +       if (bridge_bl_enable)
> +               gpiod_set_value_cansleep(bridge_bl_enable, 1);
>
>         tc35876x_brightness_control(dev, dev_priv->brightness_adjusted);
>  }
> @@ -635,8 +629,6 @@ static int tc35876x_get_panel_info(struct drm_device *dev, int pipe,
>  static int tc35876x_bridge_probe(struct i2c_client *client,
>                                 const struct i2c_device_id *id)
>  {
> -       struct tc35876x_platform_data *pdata;
> -
>         dev_info(&client->dev, "%s\n", __func__);
>
>         if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> @@ -645,26 +637,23 @@ static int tc35876x_bridge_probe(struct i2c_client *client,
>                 return -ENODEV;
>         }
>
> -       pdata = dev_get_platdata(&client->dev);
> -       if (!pdata) {
> -               dev_err(&client->dev, "%s: no platform data\n", __func__);
> -               return -ENODEV;
> -       }
> +       bridge_reset = devm_gpiod_get_optional(&client->dev, "bridge-reset", GPIOD_OUT_LOW);
> +       if (IS_ERR(bridge_reset))
> +               return PTR_ERR(bridge_reset);
> +       if (bridge_reset)
> +               gpiod_set_consumer_name(bridge_reset, "tc35876x bridge reset");
>
> -       if (pdata->gpio_bridge_reset != -1) {
> -               gpio_request(pdata->gpio_bridge_reset, "tc35876x bridge reset");
> -               gpio_direction_output(pdata->gpio_bridge_reset, 0);
> -       }
> -
> -       if (pdata->gpio_panel_bl_en != -1) {
> -               gpio_request(pdata->gpio_panel_bl_en, "tc35876x panel bl en");
> -               gpio_direction_output(pdata->gpio_panel_bl_en, 0);
> -       }
> +       bridge_bl_enable = devm_gpiod_get_optional(&client->dev, "bl-en", GPIOD_OUT_LOW);
> +       if (IS_ERR(bridge_bl_enable))
> +               return PTR_ERR(bridge_bl_enable);
> +       if (bridge_bl_enable)
> +               gpiod_set_consumer_name(bridge_bl_enable, "tc35876x panel bl en");
>
> -       if (pdata->gpio_panel_vadd != -1) {
> -               gpio_request(pdata->gpio_panel_vadd, "tc35876x panel vadd");
> -               gpio_direction_output(pdata->gpio_panel_vadd, 0);
> -       }
> +       backlight_voltage = devm_gpiod_get_optional(&client->dev, "vadd", GPIOD_OUT_LOW);
> +       if (IS_ERR(backlight_voltage))
> +               return PTR_ERR(backlight_voltage);
> +       if (backlight_voltage)
> +               gpiod_set_consumer_name(backlight_voltage, "tc35876x panel vadd");
>
>         tc35876x_client = client;
>
> @@ -673,19 +662,8 @@ static int tc35876x_bridge_probe(struct i2c_client *client,
>
>  static int tc35876x_bridge_remove(struct i2c_client *client)
>  {
> -       struct tc35876x_platform_data *pdata = dev_get_platdata(&client->dev);
> -
>         dev_dbg(&client->dev, "%s\n", __func__);
>
> -       if (pdata->gpio_bridge_reset != -1)
> -               gpio_free(pdata->gpio_bridge_reset);
> -
> -       if (pdata->gpio_panel_bl_en != -1)
> -               gpio_free(pdata->gpio_panel_bl_en);
> -
> -       if (pdata->gpio_panel_vadd != -1)
> -               gpio_free(pdata->gpio_panel_vadd);
> -
>         tc35876x_client = NULL;
>
>         return 0;
> diff --git a/include/linux/platform_data/tc35876x.h b/include/linux/platform_data/tc35876x.h
> deleted file mode 100644
> index cd6a51c71e7e..000000000000
> --- a/include/linux/platform_data/tc35876x.h
> +++ /dev/null
> @@ -1,11 +0,0 @@
> -
> -#ifndef _TC35876X_H
> -#define _TC35876X_H
> -
> -struct tc35876x_platform_data {
> -       int gpio_bridge_reset;
> -       int gpio_panel_bl_en;
> -       int gpio_panel_vadd;
> -};
> -
> -#endif /* _TC35876X_H */
> --
> 2.23.0
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-12-09 10:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-06  9:43 [PATCH] drm/gma500: Pass GPIO for Intel MID using descriptors Linus Walleij
2019-12-06  9:43 ` Linus Walleij
2019-12-08 10:20 ` Andy Shevchenko
2019-12-08 10:20   ` Andy Shevchenko
2019-12-09 10:32 ` Patrik Jakobsson
2019-12-09 10:32   ` Patrik Jakobsson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.