Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v4 3/4] drm/bridge: Add ti-tfp410 DVI transmitter driver
From: Jani Nikula @ 2016-11-23 11:08 UTC (permalink / raw)
  To: Jyri Sarha, dri-devel, devicetree
  Cc: khilman, tomi.valkeinen, laurent.pinchart, bgolaszewski
In-Reply-To: <b49e6517-2e44-c2b9-830c-ed71bb92b85b@ti.com>

On Wed, 23 Nov 2016, Jyri Sarha <jsarha@ti.com> wrote:
> On 11/22/16 16:49, Jyri Sarha wrote:
>> Add very basic ti-tfp410 DVI transmitter driver. The only feature
>> separating this from a completely dummy bridge is the EDID read
>> support trough DDC I2C. Even that functionality should be in a
>> separate generic connector driver. However, because of missing DRM
>> infrastructure support the connector is implemented within the bridge
>> driver. Some tfp410 HW specific features may be added later if needed,
>> because there is a set of registers behind i2c if it is connected.
>> 
>> This implementation is tested against my new tilcdc bridge support
>> and it works with BeagleBone DVI-D Cape Rev A3. A DT binding document
>> is also updated.
>> 
>> Signed-off-by: Jyri Sarha <jsarha@ti.com>
>> ---
>>  .../bindings/display/bridge/ti,tfp410.txt          |   9 +-
>>  drivers/gpu/drm/bridge/Kconfig                     |   7 +
>>  drivers/gpu/drm/bridge/Makefile                    |   1 +
>>  drivers/gpu/drm/bridge/ti-tfp410.c                 | 311 +++++++++++++++++++++
>>  4 files changed, 326 insertions(+), 2 deletions(-)
>>  create mode 100644 drivers/gpu/drm/bridge/ti-tfp410.c
>> 
>> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
>> index 2cbe32a..54d7e31 100644
>> --- a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
>> +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
>> @@ -6,10 +6,15 @@ Required properties:
>>  
>>  Optional properties:
>>  - powerdown-gpios: power-down gpio
>> +- reg: I2C address. If and only if present the device node
>> +       should be placed into the i2c controller node where the
>> +       tfp410 i2c is connected to.
>>  
>>  Required nodes:
>> -- Video port 0 for DPI input
>> -- Video port 1 for DVI output
>> +- Video port 0 for DPI input [1].
>> +- Video port 1 for DVI output [1].
>> +
>> +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
>>  
>>  Example
>>  -------
>> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
>> index bd6acc8..a424e03 100644
>> --- a/drivers/gpu/drm/bridge/Kconfig
>> +++ b/drivers/gpu/drm/bridge/Kconfig
>> @@ -81,6 +81,13 @@ config DRM_TOSHIBA_TC358767
>>  	---help---
>>  	  Toshiba TC358767 eDP bridge chip driver.
>>  
>> +config DRM_TI_TFP410
>> +	tristate "TI TFP410 DVI/HDMI bridge"
>> +	depends on OF
>> +	select DRM_KMS_HELPER
>> +	---help---
>> +	  Texas Instruments TFP410 DVI/HDMI Transmitter driver
>> +
>>  source "drivers/gpu/drm/bridge/analogix/Kconfig"
>>  
>>  source "drivers/gpu/drm/bridge/adv7511/Kconfig"
>> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
>> index 97ed1a5..8b065d9 100644
>> --- a/drivers/gpu/drm/bridge/Makefile
>> +++ b/drivers/gpu/drm/bridge/Makefile
>> @@ -11,3 +11,4 @@ obj-$(CONFIG_DRM_SII902X) += sii902x.o
>>  obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
>>  obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
>>  obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
>> +obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
>> diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c
>> new file mode 100644
>> index 0000000..58e26cc
>> --- /dev/null
>> +++ b/drivers/gpu/drm/bridge/ti-tfp410.c
>> @@ -0,0 +1,311 @@
>> +/*
>> + * Copyright (C) 2016 Texas Instruments
>> + * Author: Jyri Sarha <jsarha@ti.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License version 2 as published by
>> + * the Free Software Foundation.
>> + *
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/of_graph.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/i2c.h>
>> +
>> +#include <drm/drmP.h>
>> +#include <drm/drm_atomic_helper.h>
>> +#include <drm/drm_crtc.h>
>> +#include <drm/drm_crtc_helper.h>
>> +
>> +struct tfp410 {
>> +	struct drm_bridge	bridge;
>> +	struct drm_connector	connector;
>> +
>> +	struct i2c_adapter	*ddc;
>> +
>> +	struct device *dev;
>> +};
>> +
>> +static inline struct tfp410 *
>> +drm_bridge_to_tfp410(struct drm_bridge *bridge)
>> +{
>> +	return container_of(bridge, struct tfp410, bridge);
>> +}
>> +
>> +static inline struct tfp410 *
>> +drm_connector_to_tfp410(struct drm_connector *connector)
>> +{
>> +	return container_of(connector, struct tfp410, connector);
>> +}
>> +
>> +static int tfp410_get_modes(struct drm_connector *connector)
>> +{
>> +	struct tfp410 *dvi = drm_connector_to_tfp410(connector);
>> +	struct edid *edid;
>> +	int ret;
>> +
>> +	if (!dvi->ddc)
>> +		goto fallback;
>> +
>> +	edid = drm_get_edid(connector, dvi->ddc);
>> +	if (!edid) {
>> +		DRM_INFO("EDID read failed. Fallback to standard modes\n");
>> +		goto fallback;
>> +	}
>> +
>> +	drm_mode_connector_update_edid_property(connector, edid);
>> +
>> +	return drm_add_edid_modes(connector, edid);
>> +fallback:
>> +	/* No EDID, fallback on the XGA standard modes */
>> +	ret = drm_add_modes_noedid(connector, 1920, 1200);
>> +
>> +	/* And prefer a mode pretty much anything can handle */
>> +	drm_set_preferred_mode(connector, 1024, 768);
>> +
>> +	return ret;
>> +}
>> +
>> +static const struct drm_connector_helper_funcs tfp410_con_helper_funcs = {
>> +	.get_modes	= tfp410_get_modes,
>> +};
>> +
>> +static enum drm_connector_status
>> +tfp410_connector_detect(struct drm_connector *connector, bool force)
>> +{
>> +	struct tfp410 *dvi = drm_connector_to_tfp410(connector);
>> +
>> +	if (dvi->ddc) {
>> +		if (drm_probe_ddc(dvi->ddc))
>> +			return connector_status_connected;
>> +		else
>> +			return connector_status_disconnected;
>> +	}
>> +
>> +	return connector_status_unknown;
>> +}
>> +
>> +static const struct drm_connector_funcs tfp410_con_funcs = {
>> +	.dpms			= drm_atomic_helper_connector_dpms,
>> +	.detect			= tfp410_connector_detect,
>> +	.fill_modes		= drm_helper_probe_single_connector_modes,
>> +	.destroy		= drm_connector_cleanup,
>> +	.reset			= drm_atomic_helper_connector_reset,
>> +	.atomic_duplicate_state	= drm_atomic_helper_connector_duplicate_state,
>> +	.atomic_destroy_state	= drm_atomic_helper_connector_destroy_state,
>> +};
>> +
>> +static int tfp410_attach(struct drm_bridge *bridge)
>> +{
>> +	struct tfp410 *dvi = drm_bridge_to_tfp410(bridge);
>> +	int ret;
>> +
>> +	if (!bridge->encoder) {
>> +		dev_err(dvi->dev, "Missing encoder\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	drm_connector_helper_add(&dvi->connector,
>> +				 &tfp410_con_helper_funcs);
>> +	ret = drm_connector_init(bridge->dev, &dvi->connector,
>> +				 &tfp410_con_funcs, DRM_MODE_CONNECTOR_HDMIA);
>> +	if (ret) {
>> +		dev_err(dvi->dev, "drm_connector_init() failed: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	drm_mode_connector_attach_encoder(&dvi->connector,
>> +					  bridge->encoder);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct drm_bridge_funcs tfp410_bridge_funcs = {
>> +	.attach		= tfp410_attach,
>> +};
>> +
>> +static int tfp410_get_connector_ddc(struct tfp410 *dvi)
>> +{
>> +	struct device_node *ep = NULL, *connector_node = NULL;
>> +	struct device_node *ddc_phandle = NULL;
>> +	int ret = 0;
>> +
>> +	/* port@1 is the connector node */
>> +	ep = of_graph_get_endpoint_by_regs(dvi->dev->of_node, 1, -1);
>> +	if (!ep)
>> +		goto fail;
>> +
>> +	connector_node = of_graph_get_remote_port_parent(ep);
>> +	if (!connector_node)
>> +		goto fail;
>> +
>> +	ddc_phandle = of_parse_phandle(connector_node, "ddc-i2c-bus", 0);
>> +	if (!ddc_phandle)
>> +		goto fail;
>> +
>> +	dvi->ddc = of_get_i2c_adapter_by_node(ddc_phandle);
>> +	if (dvi->ddc)
>> +		dev_info(dvi->dev, "Connector's ddc i2c bus found\n");
>> +	else
>> +		ret = -EPROBE_DEFER;
>> +
>> +fail:
>> +	of_node_put(ep);
>> +	of_node_put(connector_node);
>> +	of_node_put(ddc_phandle);
>> +	return ret;
>> +}
>> +
>> +static int tfp410_init(struct device *dev)
>> +{
>> +	struct tfp410 *dvi;
>> +	int ret;
>> +
>> +	if (!dev->of_node) {
>> +		dev_err(dev, "device-tree data is missing\n");
>> +		return -ENXIO;
>> +	}
>> +
>> +	dvi = devm_kzalloc(dev, sizeof(*dvi), GFP_KERNEL);
>> +	if (!dvi)
>> +		return -ENOMEM;
>> +	dev_set_drvdata(dev, dvi);
>> +
>> +	dvi->bridge.funcs = &tfp410_bridge_funcs;
>> +	dvi->bridge.of_node = dev->of_node;
>> +	dvi->dev = dev;
>> +
>> +	ret = tfp410_get_connector_ddc(dvi);
>> +	if (ret)
>> +		goto fail;
>> +
>> +	ret = drm_bridge_add(&dvi->bridge);
>> +	if (ret) {
>> +		dev_err(dev, "drm_bridge_add() failed: %d\n", ret);
>> +		goto fail;
>> +	}
>> +
>> +	return 0;
>> +fail:
>> +	i2c_put_adapter(dvi->ddc);
>> +	return ret;
>> +}
>> +
>> +static int tfp410_fini(struct device *dev)
>> +{
>> +	struct tfp410 *dvi = dev_get_drvdata(dev);
>> +
>> +	drm_bridge_remove(&dvi->bridge);
>> +
>> +	if (dvi->ddc)
>> +		i2c_put_adapter(dvi->ddc);
>> +
>> +	return 0;
>> +}
>> +
>> +static int tfp410_probe(struct platform_device *pdev)
>> +{
>> +	return tfp410_init(&pdev->dev);
>> +}
>> +
>> +static int tfp410_remove(struct platform_device *pdev)
>> +{
>> +	return tfp410_fini(&pdev->dev);
>> +}
>> +
>
> Argh... I think I should still put all i2c code between
> #if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)

ITYM

#if IS_ENABLED(CONFIG_I2C)

BR,
Jani.


>
> #endif
>
> Otherwise I think this is now finally ready.
>
>> +/* There is currently no i2c functionality. */
>> +static int tfp410_i2c_probe(struct i2c_client *client,
>> +			    const struct i2c_device_id *id)
>> +{
>> +	int reg;
>> +
>> +	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;
>> +	}
>> +
>> +	return tfp410_init(&client->dev);
>> +}
>> +
>> +static int tfp410_i2c_remove(struct i2c_client *client)
>> +{
>> +	return tfp410_fini(&client->dev);
>> +}
>> +
>> +static const struct of_device_id tfp410_match[] = {
>> +	{ .compatible = "ti,tfp410" },
>> +	{},
>> +};
>> +MODULE_DEVICE_TABLE(of, tfp410_match);
>> +
>> +struct platform_driver tfp410_platform_driver = {
>> +	.probe	= tfp410_probe,
>> +	.remove	= tfp410_remove,
>> +	.driver	= {
>> +		.name		= "tfp410-bridge",
>> +		.of_match_table	= tfp410_match,
>> +	},
>> +};
>> +
>> +static const struct i2c_device_id tfp410_i2c_ids[] = {
>> +	{ "tfp410", 0 },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(i2c, tfp410_i2c_ids);
>> +
>> +static struct i2c_driver tfp410_i2c_driver = {
>> +	.driver = {
>> +		.name	= "tfp410",
>> +		.of_match_table = of_match_ptr(tfp410_match),
>> +	},
>> +	.id_table	= tfp410_i2c_ids,
>> +	.probe		= tfp410_i2c_probe,
>> +	.remove		= tfp410_i2c_remove,
>> +};
>> +
>> +static struct {
>> +	uint i2c:1;
>> +	uint platform:1;
>> +}  tfp410_registered_driver;
>> +
>> +static int __init tfp410_module_init(void)
>> +{
>> +	int ret;
>> +
>> +	ret = i2c_add_driver(&tfp410_i2c_driver);
>> +	if (ret)
>> +		pr_err("%s: registering i2c driver failed: %d",
>> +		       __func__, ret);
>> +	else
>> +		tfp410_registered_driver.i2c = 1;
>> +
>> +	ret = platform_driver_register(&tfp410_platform_driver);
>> +	if (ret)
>> +		pr_err("%s: registering platform driver failed: %d",
>> +		       __func__, ret);
>> +	else
>> +		tfp410_registered_driver.platform = 1;
>> +
>> +	if (tfp410_registered_driver.i2c ||
>> +	    tfp410_registered_driver.platform)
>> +		return 0;
>> +
>> +	return ret;
>> +}
>> +module_init(tfp410_module_init);
>> +
>> +static void __exit tfp410_module_exit(void)
>> +{
>> +	if (tfp410_registered_driver.i2c)
>> +		i2c_del_driver(&tfp410_i2c_driver);
>> +	if (tfp410_registered_driver.platform)
>> +		platform_driver_unregister(&tfp410_platform_driver);
>> +}
>> +module_exit(tfp410_module_exit);
>> +
>> +MODULE_AUTHOR("Jyri Sarha <jsarha@ti.com>");
>> +MODULE_DESCRIPTION("TI TFP410 DVI bridge driver");
>> +MODULE_LICENSE("GPL");
>> 
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* [PATCH v3 3/3] bus: da8xx-mstpri: fix a typo
From: Bartosz Golaszewski @ 2016-11-23 11:06 UTC (permalink / raw)
  To: Kevin Hilman, Michael Turquette, Sekhar Nori, Rob Herring,
	Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King
  Cc: linux-devicetree, LKML, linux-drm, Bartosz Golaszewski,
	Tomi Valkeinen, Jyri Sarha, Sudeep Holla, Robin Murphy, arm-soc,
	Laurent Pinchart
In-Reply-To: <1479899187-10199-1-git-send-email-bgolaszewski@baylibre.com>

Should have been priorities.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/bus/da8xx-mstpri.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bus/da8xx-mstpri.c b/drivers/bus/da8xx-mstpri.c
index 064eeb9..b17ba97 100644
--- a/drivers/bus/da8xx-mstpri.c
+++ b/drivers/bus/da8xx-mstpri.c
@@ -245,7 +245,7 @@ static int da8xx_mstpri_probe(struct platform_device *pdev)
 
 	prio_list = da8xx_mstpri_get_board_prio();
 	if (!prio_list) {
-		dev_err(dev, "no master priotities defined for board '%s'\n",
+		dev_err(dev, "no master priorities defined for board '%s'\n",
 			da8xx_mstpri_machine_get_compatible());
 		return -EINVAL;
 	}
-- 
2.9.3

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

^ permalink raw reply related

* [PATCH v3 2/3] memory: da8xx-ddrctl: drop the call to of_flat_dt_get_machine_name()
From: Bartosz Golaszewski @ 2016-11-23 11:06 UTC (permalink / raw)
  To: Kevin Hilman, Michael Turquette, Sekhar Nori, Rob Herring,
	Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King
  Cc: LKML, arm-soc, linux-drm, linux-devicetree, Jyri Sarha,
	Tomi Valkeinen, David Airlie, Laurent Pinchart, Robin Murphy,
	Sudeep Holla, Bartosz Golaszewski
In-Reply-To: <1479899187-10199-1-git-send-email-bgolaszewski@baylibre.com>

In order to avoid a section mismatch use a locally implemented routine
instead of of_flat_dt_get_machine_name() when printing the error
message.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/memory/da8xx-ddrctl.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/memory/da8xx-ddrctl.c b/drivers/memory/da8xx-ddrctl.c
index a20e7bb..1b962ee 100644
--- a/drivers/memory/da8xx-ddrctl.c
+++ b/drivers/memory/da8xx-ddrctl.c
@@ -14,7 +14,6 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
-#include <linux/of_fdt.h>
 #include <linux/platform_device.h>
 #include <linux/io.h>
 
@@ -71,6 +70,26 @@ static const struct da8xx_ddrctl_board_settings da8xx_ddrctl_board_confs[] = {
 	},
 };
 
+/*
+ * FIXME Remove this function once of/base gets a general routine for getting
+ * the machine model/compatible string.
+ */
+static const char *da8xx_ddrctl_machine_get_compatible(void)
+{
+	struct device_node *root;
+	const char *compatible;
+	int ret = -1;
+
+	root = of_find_node_by_path("/");
+	if (root) {
+		ret = of_property_read_string_index(root, "compatible",
+						    0, &compatible);
+		of_node_put(root);
+	}
+
+	return ret ? NULL : compatible;
+}
+
 static const struct da8xx_ddrctl_config_knob *
 da8xx_ddrctl_match_knob(const struct da8xx_ddrctl_setting *setting)
 {
@@ -118,7 +137,7 @@ static int da8xx_ddrctl_probe(struct platform_device *pdev)
 	setting = da8xx_ddrctl_get_board_settings();
 	if (!setting) {
 		dev_err(dev, "no settings for board '%s'\n",
-			of_flat_dt_get_machine_name());
+			da8xx_ddrctl_machine_get_compatible());
 		return -EINVAL;
 	}
 
-- 
2.9.3

^ permalink raw reply related

* [PATCH v3 1/3] bus: da8xx-mstpri: drop the call to of_flat_dt_get_machine_name()
From: Bartosz Golaszewski @ 2016-11-23 11:06 UTC (permalink / raw)
  To: Kevin Hilman, Michael Turquette, Sekhar Nori, Rob Herring,
	Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King
  Cc: linux-devicetree, LKML, linux-drm, Bartosz Golaszewski,
	Tomi Valkeinen, Jyri Sarha, Sudeep Holla, Robin Murphy, arm-soc,
	Laurent Pinchart
In-Reply-To: <1479899187-10199-1-git-send-email-bgolaszewski@baylibre.com>

In order to avoid a section mismatch use a locally implemented routine
instead of of_flat_dt_get_machine_name() when printing the error
message.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/bus/da8xx-mstpri.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/bus/da8xx-mstpri.c b/drivers/bus/da8xx-mstpri.c
index 85f0b53..064eeb9 100644
--- a/drivers/bus/da8xx-mstpri.c
+++ b/drivers/bus/da8xx-mstpri.c
@@ -16,7 +16,6 @@
 #include <linux/platform_device.h>
 #include <linux/io.h>
 #include <linux/regmap.h>
-#include <linux/of_fdt.h>
 
 /*
  * REVISIT: Linux doesn't have a good framework for the kind of performance
@@ -190,6 +189,26 @@ static const struct da8xx_mstpri_board_priorities da8xx_mstpri_board_confs[] = {
 	},
 };
 
+/*
+ * FIXME Remove this function once of/base gets a general routine for getting
+ * the machine model/compatible string.
+ */
+static const char *da8xx_mstpri_machine_get_compatible(void)
+{
+	struct device_node *root;
+	const char *compatible;
+	int ret = -1;
+
+	root = of_find_node_by_path("/");
+	if (root) {
+		ret = of_property_read_string_index(root, "compatible",
+						    0, &compatible);
+		of_node_put(root);
+	}
+
+	return ret ? NULL : compatible;
+}
+
 static const struct da8xx_mstpri_board_priorities *
 da8xx_mstpri_get_board_prio(void)
 {
@@ -227,7 +246,7 @@ static int da8xx_mstpri_probe(struct platform_device *pdev)
 	prio_list = da8xx_mstpri_get_board_prio();
 	if (!prio_list) {
 		dev_err(dev, "no master priotities defined for board '%s'\n",
-			of_flat_dt_get_machine_name());
+			da8xx_mstpri_machine_get_compatible());
 		return -EINVAL;
 	}
 
-- 
2.9.3

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

^ permalink raw reply related

* [PATCH v3 0/3] da8xx: fix section mismatch in new drivers
From: Bartosz Golaszewski @ 2016-11-23 11:06 UTC (permalink / raw)
  To: Kevin Hilman, Michael Turquette, Sekhar Nori, Rob Herring,
	Frank Rowand, Mark Rutland, Peter Ujfalusi, Russell King
  Cc: LKML, arm-soc, linux-drm, linux-devicetree, Jyri Sarha,
	Tomi Valkeinen, David Airlie, Laurent Pinchart, Robin Murphy,
	Sudeep Holla, Bartosz Golaszewski

Sekhar noticed there's a section mismatch in the da8xx-mstpri and
da8xx-ddrctl drivers. This is caused by calling
of_flat_dt_get_machine_name() which has an __init annotation.

This series addresses this issue by open coding routines that return
the machine compatible string in both drivers. Once a general function
for that in of/base is merged, we'll remove them.

The third patch fixes a typo that got in last time.

v1 -> v2:
- drop patch [1/3] from v1
- introduce internal routines in the drivers instead of a general
  function in of/base.c

v2 -> v3:
- use of_property_read_string_index() instead of
  of_property_read_string() to get the first compatible entry
- s/priotities/priorities

Bartosz Golaszewski (3):
  bus: da8xx-mstpri: drop the call to of_flat_dt_get_machine_name()
  memory: da8xx-ddrctl: drop the call to of_flat_dt_get_machine_name()
  bus: da8xx-mstpri: fix a typo

 drivers/bus/da8xx-mstpri.c    | 25 ++++++++++++++++++++++---
 drivers/memory/da8xx-ddrctl.c | 23 +++++++++++++++++++++--
 2 files changed, 43 insertions(+), 5 deletions(-)

-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 2/3] pinctrl: New driver for TI DA8XX/OMAP-L138/AM18XX pinconf
From: Sekhar Nori @ 2016-11-23 11:04 UTC (permalink / raw)
  To: David Lechner, Linus Walleij, Rob Herring, Mark Rutland,
	Kevin Hilman
  Cc: linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Axel Haslam,
	Alexandre Bailon, Bartosz Gołaszewski
In-Reply-To: <1479871767-20160-3-git-send-email-david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>

On Wednesday 23 November 2016 08:59 AM, David Lechner wrote:
> This adds a new driver for pinconf on TI DA8XX/OMAP-L138/AM18XX. These

s/DA8XX/DA850/

> SoCs have a separate controller for controlling pullup/pulldown groups.
> 
> Signed-off-by: David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>

> +static const char *da850_pupd_get_get_group_name(struct pinctrl_dev *pctldev,
> +						 unsigned int selector)
> +{
> +	return da850_pupd_group_names[selector];
> +}
> +
> +static int da850_pupd_get_get_group_pins(struct pinctrl_dev *pctldev,
> +					 unsigned int selector,
> +					 const unsigned int **pins,
> +					 unsigned int *num_pins)
> +{
> +	*num_pins = 0;
> +
> +	return 0;
> +}

usage of get_get_ in the function names above is odd.

Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH V3 2/2] regulator: Add settling time for non-linear voltage transition
From: Laxman Dewangan @ 2016-11-23 11:02 UTC (permalink / raw)
  To: broonie-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A
  Cc: mark.rutland-5wv7dgnIgG8, lgirdwood-Re5JQEeQqe8AvxtiuMwx3w,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Laxman Dewangan,
	Douglas Anderson, Aleksandr Frid
In-Reply-To: <1479898960-14166-1-git-send-email-ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

Some regulators (some PWM regulators) have the voltage transition
non-linear i.e. exponentially. On such cases, the settling time
for voltage transition can not be presented in the voltage-ramp-delay.

Add new property for non-linear voltage transition and handle this
in getting the voltage settling time.

Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
CC: Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
CC: Aleksandr Frid <afrid-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

---
This patch is continuation of discussion on patch
    regulator: pwm: Fix regulator ramp delay for continuous mode
https://patchwork.kernel.org/patch/9216857/
where is it discussed to have separate property for PWM which has
exponential voltage transition.

Changes from V1:
- Use new DT property to finding that voltage ramp is exponential or
  not and use flag for having fixed delay for all voltage change.

Changes from V2:
- Based on review comment from V1, make the settling time property
  independent of the regulator-ramp-delay and move this to core
  framework instead of handling in PWM regulator.
---
 drivers/regulator/core.c          | 2 ++
 drivers/regulator/of_regulator.c  | 4 ++++
 include/linux/regulator/machine.h | 3 +++
 3 files changed, 9 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 67426c0..06a2477 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2752,6 +2752,8 @@ static int _regulator_set_voltage_time(struct regulator_dev *rdev,
 		ramp_delay = rdev->constraints->ramp_delay;
 	else if (rdev->desc->ramp_delay)
 		ramp_delay = rdev->desc->ramp_delay;
+	else if (rdev->constraints->settling_time)
+		return rdev->constraints->settling_time;
 
 	if (ramp_delay == 0) {
 		rdev_warn(rdev, "ramp_delay not set\n");
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 4f613ec..09d677d 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -86,6 +86,10 @@ static void of_get_regulation_constraints(struct device_node *np,
 			constraints->ramp_disable = true;
 	}
 
+	ret = of_property_read_u32(np, "regulator-settling-time-us", &pval);
+	if (!ret)
+		constraints->settling_time = pval;
+
 	ret = of_property_read_u32(np, "regulator-enable-ramp-delay", &pval);
 	if (!ret)
 		constraints->enable_time = pval;
diff --git a/include/linux/regulator/machine.h b/include/linux/regulator/machine.h
index ad3e515..598a493 100644
--- a/include/linux/regulator/machine.h
+++ b/include/linux/regulator/machine.h
@@ -108,6 +108,8 @@ struct regulator_state {
  * @initial_state: Suspend state to set by default.
  * @initial_mode: Mode to set at startup.
  * @ramp_delay: Time to settle down after voltage change (unit: uV/us)
+ * @settling_time: Time to settle down after voltage change when voltage
+ *		   change is non-linear (unit: microseconds).
  * @active_discharge: Enable/disable active discharge. The enum
  *		      regulator_active_discharge values are used for
  *		      initialisation.
@@ -149,6 +151,7 @@ struct regulation_constraints {
 	unsigned int initial_mode;
 
 	unsigned int ramp_delay;
+	unsigned int settling_time;
 	unsigned int enable_time;
 
 	unsigned int active_discharge;
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH V3 1/2] regulator: DT: Add settling time property for non-linear voltage change
From: Laxman Dewangan @ 2016-11-23 11:02 UTC (permalink / raw)
  To: broonie, robh+dt
  Cc: mark.rutland, lgirdwood, linux-kernel, devicetree,
	Laxman Dewangan, Douglas Anderson, Aleksandr Frid

Some regulators (some PWM regulators) have the voltage transition
exponentially. On such cases, the settling time for voltage change
is treated as constant time.

Add DT property for providing the settling time for any level of
voltage change for non-linear voltage change.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
CC: Douglas Anderson <dianders@chromium.org>
CC: Aleksandr Frid <afrid@nvidia.com>

---
This patch is continuation of discussion on patch
   regulator: pwm: Fix regulator ramp delay for continuous mode
   https://patchwork.kernel.org/patch/9216857/
where is it discussed to have separate property for PWM which has
exponential voltage transition.

Changes from V1:
- Pass the flag to tell that voltage ramp is exponential instead of
  providing delay.

Changes from V2:
- Based on review comment from V1, make the settling time property
  independent of the regulator-ramp-delay and move this out of PWM
  regulator.
---
 Documentation/devicetree/bindings/regulator/regulator.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
index 6ab5aef..d18edb0 100644
--- a/Documentation/devicetree/bindings/regulator/regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/regulator.txt
@@ -21,6 +21,9 @@ Optional properties:
   design requires. This property describes the total system ramp time
   required due to the combination of internal ramping of the regulator itself,
   and board design issues such as trace capacitance and load on the supply.
+- regulator-settling-time-us: Settling time, in microseconds, for voltage
+  change if regulator have the constant time for any level voltage change.
+  This is useful when regulator have exponential voltage change.
 - regulator-soft-start: Enable soft start so that voltage ramps slowly
 - regulator-state-mem sub-root node for Suspend-to-RAM mode
   : suspend to memory, the device goes to sleep, but all data stored in memory,
-- 
2.1.4

^ permalink raw reply related

* Re: [PATCH 1/3] devicetree: bindings: pinctrl: Add binding for ti,da850-pupd
From: Sekhar Nori @ 2016-11-23 11:01 UTC (permalink / raw)
  To: David Lechner, Linus Walleij, Rob Herring, Mark Rutland,
	Kevin Hilman
  Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel,
	Axel Haslam, Alexandre Bailon, Bartosz Gołaszewski
In-Reply-To: <1479871767-20160-2-git-send-email-david@lechnology.com>

On Wednesday 23 November 2016 08:59 AM, David Lechner wrote:
> Device-tree bindings for TI DA8XX/OMAP-L138/AM18XX pullup/pulldown

s/DA8XX/DA850. It looks like this support is absent from DA830.

> pinconf controller.
> 
> Signed-off-by: David Lechner <david@lechnology.com>

Thanks,
Sekhar

^ permalink raw reply

* Re: [PATCH v2] ARM: dts: da850: add the mstpri and ddrctl nodes
From: Bartosz Golaszewski @ 2016-11-23 10:27 UTC (permalink / raw)
  To: David Lechner
  Cc: Mark Rutland, linux-devicetree, Tomi Valkeinen, Kevin Hilman,
	Michael Turquette, Sekhar Nori, Russell King, linux-drm, LKML,
	Peter Ujfalusi, Rob Herring, Jyri Sarha, Frank Rowand, arm-soc,
	Laurent Pinchart
In-Reply-To: <a39b9276-865b-6382-574e-a5ef040a452f@lechnology.com>

2016-11-22 23:23 GMT+01:00 David Lechner <david@lechnology.com>:
> On 11/15/2016 05:00 AM, Bartosz Golaszewski wrote:
>>
>> Add the nodes for the MSTPRI configuration and DDR2/mDDR memory
>> controller drivers to da850.dtsi.
>>
>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>> ---
>> v1 -> v2:
>> - moved the priority controller node above the cfgchip node
>> - renamed added nodes to better reflect their purpose
>>
>>  arch/arm/boot/dts/da850.dtsi | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>> index 1bb1f6d..412eec6 100644
>> --- a/arch/arm/boot/dts/da850.dtsi
>> +++ b/arch/arm/boot/dts/da850.dtsi
>> @@ -210,6 +210,10 @@
>>                         };
>>
>>                 };
>> +               prictrl: priority-controller@14110 {
>> +                       compatible = "ti,da850-mstpri";
>> +                       reg = <0x14110 0x0c>;
>
>
> I think we should add status = "disabled"; here and let boards opt in.
>
>> +               };
>>                 cfgchip: chip-controller@1417c {
>>                         compatible = "ti,da830-cfgchip", "syscon",
>> "simple-mfd";
>>                         reg = <0x1417c 0x14>;
>> @@ -451,4 +455,8 @@
>>                           1 0 0x68000000 0x00008000>;
>>                 status = "disabled";
>>         };
>> +       memctrl: memory-controller@b0000000 {
>> +               compatible = "ti,da850-ddr-controller";
>> +               reg = <0xb0000000 0xe8>;
>
>
> same here. status = "disabled";
>
>> +       };
>>  };
>>

Hi David,

I did that initially[1][2] and it was rejected by Kevin[3] and Laurent[4].

FYI this patch has already been queued by Sekhar.

Best regards,
Bartosz Golaszewski

[1] https://www.spinics.net/lists/arm-kernel/msg539638.html
[2] http://www.spinics.net/lists/devicetree/msg148575.html
[3] http://www.spinics.net/lists/devicetree/msg148667.html
[4] http://www.spinics.net/lists/devicetree/msg148655.html
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH 1/2] of: base: add support to get machine model name
From: Sudeep Holla @ 2016-11-23 10:25 UTC (permalink / raw)
  To: Rob Herring
  Cc: Frank Rowand, Sudeep Holla, linux-kernel@vger.kernel.org,
	Arnd Bergmann, devicetree@vger.kernel.org
In-Reply-To: <CAL_JsqLsw6BNSDY8pGJ8x3XF21WdciLC7WC+z796pYtDk_kvxA@mail.gmail.com>



On 22/11/16 21:35, Rob Herring wrote:
> On Tue, Nov 22, 2016 at 12:44 PM, Frank Rowand <frowand.list@gmail.com> wrote:

[...]

>>
>> This patch adds a function that leads to conflating the "model" property
>> and the "compatible" property. This leads to opaque, confusing and unclear
>> code where ever it is used.   I think it is not good for the device tree
>> framework to contribute to writing unclear code.
>>
>> Further, only two of the proposed users of this new function appear to
>> be proper usage.  I do not think that the small amount of reduced lines
>> of code is a good trade off for the reduced code clarity and for the
>> potential for future mis-use of this function.
>>
>> Can I convince you to revert this patch?
>
> Yes, I will revert.
>
>> If not, will you accept a patch to change the function name to more
>> clearly indicate what it does?  (One possible name would be
>> of_model_or_1st_compatible().)
>
> I took it as there's already the FDT equivalent function.

Yes it was mainly for non of_flat_* replacement for
of_flat_dt_get_machine_name

-- 
Regards,
Sudeep

^ permalink raw reply

* Re: [PATCH 1/2] of: base: add support to get machine model name
From: Sudeep Holla @ 2016-11-23 10:23 UTC (permalink / raw)
  To: Frank Rowand
  Cc: Rob Herring, Sudeep Holla, linux-kernel, Arnd Bergmann,
	devicetree
In-Reply-To: <5834921F.2020809@gmail.com>



On 22/11/16 18:44, Frank Rowand wrote:
> Hi Rob,

[...]

>
> This patch adds a function that leads to conflating the "model"
> property and the "compatible" property. This leads to opaque,
> confusing and unclear code where ever it is used.   I think it is
> not good for the device tree framework to contribute to writing
> unclear code.
>

I agree, the main intention of this patch initially was to have a non
flat_* version of of_flat_dt_get_machine_name

> Further, only two of the proposed users of this new function appear
> to be proper usage.  I do not think that the small amount of reduced
> lines of code is a good trade off for the reduced code clarity and
> for the potential for future mis-use of this function.
>

OK, most of the place I found it used for logging/informational purpose
and hence I thought it could replace in places where even compatible is
used. If that's wrong or leads to misuse of this API, then fine we
should not have one.

-- 
Regards,
Sudeep

^ permalink raw reply

* Re: [PATCHv2 3/4] dt: bindings: add new dt entry for BTCOEX feature in qcom,ath10k.txt
From: Tamizh chelvam @ 2016-11-23 10:20 UTC (permalink / raw)
  To: Rob Herring
  Cc: c_traja-Rm6X0d1/PG5y9aJCnZT0Uw,
	ath10k-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161118144406.3se7gnckhcmwqytp@rob-hp-laptop>

Thanks for the comments.

On 2016-11-18 20:14, Rob Herring wrote:
> On Thu, Nov 17, 2016 at 05:14:23PM +0530, c_traja-Rm6X0d1/PG5y9aJCnZT0Uw@public.gmane.org 
> wrote:
>> From: Tamizh chelvam <tamizhchelvam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> 
>> There two things done in this patch.
>> 
>> 1) 'btcoex_support' flag for BTCOEX feature support by the hardware.
>> 2) 'wlan_btcoex_gpio' is used to fill wlan priority pin number for
>>    BTCOEX priority feature support.
>> 
>> Signed-off-by: Tamizh chelvam <tamizhchelvam-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> ---
>>  .../bindings/net/wireless/qcom,ath10k.txt          |    4 ++++
>>  1 file changed, 4 insertions(+)
>> 
>> diff --git 
>> a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt 
>> b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt
>> index 74d7f0a..08150e2d 100644
>> --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt
>> +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt
>> @@ -46,6 +46,10 @@ Optional properties:
>>  				 hw versions.
>>  - qcom,ath10k-pre-calibration-data : pre calibration data as an 
>> array,
>>  				     the length can vary between hw versions.
>> +- btcoex_support  : should contain eithr "0" or "1" to indicate 
>> btcoex
>> +		    support by the hardware.
> 
> This is BT coexistence? Make this boolean and n

Yes, this is BT coexistence. And I didn't get what are you trying to say 
in this "Make this boolean and n"
> 
>> +- btcoex_gpio_pin :  btcoex gpio pin number for the device which
>> +		     supports BTCOEX.
> 
> This is a pin number on the chip, not any pin number Linux GPIO subsys
> cares about, right? Is there a connection to the host too, or this is
> internal between BT and WiFi?

This is internal between BT and wifi.
> 
> Do you really need 2 properties? Does supporting this feature require
> the GPIO? If so, then the first property is redundant.
> 
Target/driver can hard copy this gpio pin for some chipsets and there we 
will need btcoex_support flag to find the btcoex support.

> Needs vendor prefix and don't use '_'. Should be something like
> 'qcom,bt-coexist-gpio-pin'.
> 
Sure I'll update this and send in v3 patch

^ permalink raw reply

* Re: [PATCH v2 1/5] ARM: memory: da8xx-ddrctl: new driver
From: Sudeep Holla @ 2016-11-23 10:16 UTC (permalink / raw)
  To: Frank Rowand
  Cc: Sekhar Nori, Bartosz Golaszewski, Kevin Hilman, Michael Turquette,
	Rob Herring, Mark Rutland, Peter Ujfalusi, Russell King,
	Sudeep Holla, LKML, arm-soc, linux-drm, linux-devicetree,
	Jyri Sarha, Tomi Valkeinen, David Airlie, Laurent Pinchart
In-Reply-To: <5833A2DA.40701@gmail.com>



On 22/11/16 01:43, Frank Rowand wrote:
> Hi Sekhar,
>
> (And adding Sudeep since he becomes involved in this further
> down thread and at that point says he will re-work this
> proposed work around in a manner that is incorrect in a
> manner that is similar to this proposed work around.)
>
> On 11/21/16 08:33, Sekhar Nori wrote:


[...]

>>  static int da8xx_ddrctl_probe(struct platform_device *pdev)
>>  {
>>  	const struct da8xx_ddrctl_config_knob *knob;
>> @@ -118,7 +130,7 @@ static int da8xx_ddrctl_probe(struct platform_device *pdev)
>>  	setting = da8xx_ddrctl_get_board_settings();
>>  	if (!setting) {
>>  		dev_err(dev, "no settings for board '%s'\n",
>> -			of_flat_dt_get_machine_name());
>
> da8xx_ddrctl_get_board_settings() tries to match based on the "compatible"
> property in the root node.  The "model" property in the root node has
> nothing to do with the failure to match. So creating and then using
> da8xx_ddrctl_get_machine_name() to potentially report model is not useful.
>
> It should be sufficient to simply report that no compatible matched.
>

Agreed.

-- 
Regards,
Sudeep

^ permalink raw reply

* [PATCH] v4l: async: make v4l2 coexists with devicetree nodes in a dt overlay
From: Javi Merino @ 2016-11-23 10:09 UTC (permalink / raw)
  To: linux-media
  Cc: linux-kernel, devicetree, Pantelis Antoniou, Javi Merino,
	Mauro Carvalho Chehab, Javier Martinez Canillas, Sakari Ailus

In asd's configured with V4L2_ASYNC_MATCH_OF, if the v4l2 subdev is in
a devicetree overlay, its of_node pointer will be different each time
the overlay is applied.  We are not interested in matching the
pointer, what we want to match is that the path is the one we are
expecting.  Change to use of_node_cmp() so that we continue matching
after the overlay has been removed and reapplied.

Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Javier Martinez Canillas <javier@osg.samsung.com>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Javi Merino <javi.merino@kernel.org>
---
Hi,

I feel it is a bit of a hack, but I couldn't think of anything better.
I'm ccing devicetree@ and Pantelis because there may be a simpler
solution.

 drivers/media/v4l2-core/v4l2-async.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
index 5bada20..d33a17c 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -42,7 +42,8 @@ static bool match_devname(struct v4l2_subdev *sd,
 
 static bool match_of(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
 {
-	return sd->of_node == asd->match.of.node;
+	return !of_node_cmp(of_node_full_name(sd->of_node),
+			    of_node_full_name(asd->match.of.node));
 }
 
 static bool match_custom(struct v4l2_subdev *sd, struct v4l2_async_subdev *asd)
-- 
2.1.4

^ permalink raw reply related

* Re: [PATCH 1/3] of: base: add support to get machine compatible string
From: Sudeep Holla @ 2016-11-23 10:05 UTC (permalink / raw)
  To: Sekhar Nori
  Cc: Mark Rutland, linux-devicetree, Tomi Valkeinen, David Airlie,
	Kevin Hilman, Michael Turquette, Russell King, linux-drm, LKML,
	Bartosz Golaszewski, Rob Herring, Jyri Sarha, Sudeep Holla,
	Robin Murphy, Frank Rowand, arm-soc, Laurent Pinchart
In-Reply-To: <ae92e013-b41b-6caa-b32f-284ffb6f5aa0@ti.com>



On 23/11/16 07:49, Sekhar Nori wrote:
> On Tuesday 22 November 2016 09:16 PM, Sudeep Holla wrote:
>> Hi Sekhar,
>>
>> On 22/11/16 15:06, Sekhar Nori wrote:
>>> Hi Sudeep,
>>>
>>> On Tuesday 22 November 2016 04:23 PM, Sudeep Holla wrote:
>>>>
>>>>
>>>> On 22/11/16 10:41, Bartosz Golaszewski wrote:
>>>>> Add a function allowing to retrieve the compatible string of the root
>>>>> node of the device tree.
>>>>>
>>>>
>>>> Rob has queued [1] and it's in -next today. You can reuse that if you
>>>> are planning to target this for v4.11 or just use open coding in your
>>>> driver for v4.10 and target this move for v4.11 to avoid cross tree
>>>> dependencies as I already mentioned in your previous thread.
>>>
>>> I dont have your original patch in my mailbox, but I wonder if
>>> returning a pointer to property string for a node whose reference has
>>> already been released is safe to do? Probably not an issue for the root
>>> node, but still feels counter-intuitive.
>>>
>>
>> I am not sure if I understand the issue here. Are you referring a case
>> where of_root is freed ?
>
> Yes, right, thats what I was hinting at. Since you are giving up the
> reference to the device node before the function returns, the user can
> be left with a dangling reference.
>

Yes I agree.

>> Also I have seen drivers today just using this pointer directly, but
>> it's better to copy the string(I just saw this done in one case)
>
> Hmm, the reference is given up before the API returns, so I doubt
> copying it later is any additional benefit.
>

True.

> I suspect this is a theoretical issue though since root device node is
> probably never freed.
>

Indeed, not sure if it's worth adding additional code to release the nod
at all call sites.

-- 
Regards,
Sudeep

^ permalink raw reply

* Re: [PATCH 7/7] add stm32 multi-functions timer driver in DT
From: Lee Jones @ 2016-11-23  9:53 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: mark.rutland, devicetree, lars, alexandre.torgue, linux-pwm,
	linux-iio, linus.walleij, arnaud.pouliquen, linux-kernel, robh+dt,
	thierry.reding, linux-arm-kernel, pmeerw, knaack.h, gerald.baeza,
	fabrice.gasnier, linaro-kernel, jic23, Benjamin Gaignard
In-Reply-To: <1479831207-32699-8-git-send-email-benjamin.gaignard@st.com>

On Tue, 22 Nov 2016, Benjamin Gaignard wrote:

> Add timers MFD and childs into DT for stm32f4.
> Define and enable pwm1 and pwm3 for stm32f469 discovery board
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> ---
>  arch/arm/boot/dts/stm32f429.dtsi      | 246 ++++++++++++++++++++++++++++++++++
>  arch/arm/boot/dts/stm32f469-disco.dts |  29 ++++
>  2 files changed, 275 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
> index bca491d..28a0fe9 100644
> --- a/arch/arm/boot/dts/stm32f429.dtsi
> +++ b/arch/arm/boot/dts/stm32f429.dtsi
> @@ -355,6 +355,21 @@
>  					slew-rate = <2>;
>  				};
>  			};
> +
> +			pwm1_pins: pwm@1 {
> +				pins {
> +					pinmux = <STM32F429_PA8_FUNC_TIM1_CH1>,
> +						 <STM32F429_PB13_FUNC_TIM1_CH1N>,
> +						 <STM32F429_PB12_FUNC_TIM1_BKIN>;
> +				};
> +			};
> +
> +			pwm3_pins: pwm@3 {
> +				pins {
> +					pinmux = <STM32F429_PB4_FUNC_TIM3_CH1>,
> +						 <STM32F429_PB5_FUNC_TIM3_CH2>;
> +				};
> +			};
>  		};
>  
>  		rcc: rcc@40023810 {
> @@ -426,6 +441,237 @@
>  			interrupts = <80>;
>  			clocks = <&rcc 0 38>;
>  		};
> +
> +		mfd_timer1: mfdtimer1@40010000 {

Do you reference this node?

If not, it should read:

  advanced-control@40010000

> +			compatible = "st,stm32-mfd-timer1";

"st,stm32-advanced-control"

> +			reg = <0x40010000 0x400>;
> +			clocks = <&rcc 0 160>;
> +			clock-names = "mfd_timer_clk";

"clk_int"

> +			interrupts = <27>;

This is a timer property.

Also move the associated registration C code into the timer driver.

> +			status = "disabled";
> +
> +			pwm1: pwm1@40010000 {

  pwm@0 {

> +				compatible = "st,stm32-pwm1";

st,stm32-advanced-control-pwm

> +				status = "disabled";
> +			};
> +
> +			iiotimer1: iiotimer1@40010000 {

Same here:

  timer@0

> +				compatible = "st,stm32-iio-timer1";

st,stm32-advanced-control-timer

> +				status = "disabled";
> +			};
> +		};
> +
> +		mfd_timer2: mfdtimer2@40000000 {
> +			compatible = "st,stm32-mfd-timer2";
> +			reg = <0x40000000 0x400>;
> +			clocks = <&rcc 0 128>;
> +			clock-names = "mfd_timer_clk";
> +			interrupts = <28>;
> +			status = "disabled";
> +
> +			pwm2: pwm2@40000000 {
> +				compatible = "st,stm32-pwm2";
> +				status = "disabled";
> +			};
> +			iiotimer2: iiotimer2@40000000 {
> +				compatible = "st,stm32-iio-timer2";
> +				status = "disabled";
> +			};
> +		};
> +
> +		mfd_timer3: mfdtimer3@40000400 {
> +			compatible = "st,stm32-mfd-timer3";
> +			reg = <0x40000400 0x400>;
> +			clocks = <&rcc 0 129>;
> +			clock-names = "mfd_timer_clk";
> +			interrupts = <29>;
> +			status = "disabled";
> +
> +			pwm3: pwm3@40000400 {
> +				compatible = "st,stm32-pwm3";
> +				status = "disabled";
> +			};
> +			iiotimer3: iiotimer3@40000400 {
> +				compatible = "st,stm32-iio-timer3";
> +				status = "disabled";
> +			};
> +		};
> +
> +		mfd_timer4: mfdtimer4@40000800 {
> +			compatible = "st,stm32-mfd-timer4";
> +			reg = <0x40000800 0x400>;
> +			clocks = <&rcc 0 130>;
> +			clock-names = "mfd_timer_clk";
> +			interrupts = <30>;
> +			status = "disabled";
> +
> +			pwm4: pwm4@40000800 {
> +				compatible = "st,stm32-pwm4";
> +				status = "disabled";
> +			};
> +			iiotimer4: iiotimer4@40000800 {
> +				compatible = "st,stm32-iio-timer4";
> +				status = "disabled";
> +			};
> +		};
> +
> +		mfd_timer5: mfdtimer5@40000C00 {
> +			compatible = "st,stm32-mfd-timer5";
> +			reg = <0x40000C00 0x400>;
> +			clocks = <&rcc 0 131>;
> +			clock-names = "mfd_timer_clk";
> +			interrupts = <50>;
> +			status = "disabled";
> +
> +			pwm5: pwm5@40000C00 {
> +				compatible = "st,stm32-pwm5";
> +				status = "disabled";
> +			};
> +			iiotimer5: iiotimer5@40000800 {
> +				compatible = "st,stm32-iio-timer5";
> +				status = "disabled";
> +			};
> +		};
> +
> +		mfd_timer6: mfdtimer6@40001000 {
> +			compatible = "st,stm32-mfd-timer6";
> +			reg = <0x40001000 0x400>;
> +			clocks = <&rcc 0 132>;
> +			clock-names = "mfd_timer_clk";
> +			interrupts = <54>;
> +			status = "disabled";
> +
> +			iiotimer6: iiotimer6@40001000 {
> +				compatible = "st,stm32-iio-timer6";
> +				status = "disabled";
> +			};
> +		};
> +
> +		mfd_timer7: mfdtimer7@40001400 {
> +			compatible = "st,stm32-mfd-timer7";
> +			reg = <0x40001400 0x400>;
> +			clocks = <&rcc 0 133>;
> +			clock-names = "mfd_timer_clk";
> +			interrupts = <55>;
> +			status = "disabled";
> +
> +			iiotimer7: iiotimer7@40001400 {
> +				compatible = "st,stm32-iio-timer7";
> +				status = "disabled";
> +			};
> +		};
> +
> +		mfd_timer8: mfdtimer8@40010400 {
> +			compatible = "st,stm32-mfd-timer8";
> +			reg = <0x40010400 0x400>;
> +			clocks = <&rcc 0 161>;
> +			clock-names = "mfd_timer_clk";
> +			interrupts = <46>;
> +			status = "disabled";
> +
> +			pwm8: pwm8@40010400 {
> +				compatible = "st,stm32-pwm8";
> +				status = "disabled";
> +			};
> +
> +			iiotimer8: iiotimer7@40010400 {
> +				compatible = "st,stm32-iio-timer8";
> +				status = "disabled";
> +			};
> +		};
> +
> +		mfd_timer9: mfdtimer9@40014000 {
> +			compatible = "st,stm32-mfd-timer9";
> +			reg = <0x40014000 0x400>;
> +			clocks = <&rcc 0 176>;
> +			clock-names = "mfd_timer_clk";
> +			interrupts = <24>;
> +			status = "disabled";
> +
> +			pwm9: pwm9@40014000 {
> +				compatible = "st,stm32-pwm9";
> +				status = "disabled";
> +			};
> +
> +			iiotimer9: iiotimer9@40014000 {
> +				compatible = "st,stm32-iio-timer9";
> +				status = "disabled";
> +			};
> +		};
> +
> +		mfd_timer10: mfdtimer10@40014400 {
> +			compatible = "st,stm32-mfd-timer10";
> +			reg = <0x40014400 0x400>;
> +			clocks = <&rcc 0 177>;
> +			clock-names = "mfd_timer_clk";
> +			interrupts = <25>;
> +			status = "disabled";
> +
> +			pwm10: pwm10@40014400 {
> +				compatible = "st,stm32-pwm10";
> +				status = "disabled";
> +			};
> +		};
> +
> +		mfd_timer11: mfdtimer11@40014800 {
> +			compatible = "st,stm32-mfd-timer11";
> +			reg = <0x40014800 0x400>;
> +			clocks = <&rcc 0 178>;
> +			clock-names = "mfd_timer_clk";
> +			interrupts = <26>;
> +			status = "disabled";
> +
> +			pwm11: pwm11@40014800 {
> +				compatible = "st,stm32-pwm11";
> +				status = "disabled";
> +			};
> +		};
> +
> +		mfd_timer12: mfdtimer12@40001800 {
> +			compatible = "st,stm32-mfd-timer12";
> +			reg = <0x40001800 0x400>;
> +			clocks = <&rcc 0 134>;
> +			clock-names = "mfd_timer_clk";
> +			interrupts = <43>;
> +			status = "disabled";
> +
> +			pwm12: pwm12@40001800 {
> +				compatible = "st,stm32-pwm12";
> +				status = "disabled";
> +			};
> +			iiotimer12: iiotimer12@40001800 {
> +				compatible = "st,stm32-iio-timer12";
> +				status = "disabled";
> +			};
> +		};
> +
> +		mfd_timer13: mfdtimer13@40001C00 {
> +			compatible = "st,stm32-mfd-timer13";
> +			reg = <0x40001C00 0x400>;
> +			clocks = <&rcc 0 135>;
> +			clock-names = "mfd_timer_clk";
> +			interrupts = <44>;
> +			status = "disabled";
> +
> +			pwm13: pwm13@40001C00 {
> +				compatible = "st,stm32-pwm13";
> +				status = "disabled";
> +			};
> +		};
> +
> +		mfd_timer14: mfdtimer14@40002000 {
> +			compatible = "st,stm32-mfd-timer14";
> +			reg = <0x40002000 0x400>;
> +			clocks = <&rcc 0 136>;
> +			clock-names = "mfd_timer_clk";
> +			interrupts = <45>;
> +			status = "disabled";
> +
> +			pwm14: pwm14@40002000 {
> +				compatible = "st,stm32-pwm14";
> +				status = "disabled";
> +			};
> +		};
>  	};
>  };
>  
> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts b/arch/arm/boot/dts/stm32f469-disco.dts
> index 8a163d7..a8f1788 100644
> --- a/arch/arm/boot/dts/stm32f469-disco.dts
> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
> @@ -81,3 +81,32 @@
>  &usart3 {
>  	status = "okay";
>  };
> +
> +&mfd_timer1 {
> +	status = "okay";
> +};
> +
> +&pwm1 {
> +	pinctrl-0	= <&pwm1_pins>;
> +	pinctrl-names	= "default";
> +	st,breakinput-polarity = <0>;

Is this documented?

I'm sure we have generic polarity properties somewhere already?

> +	status = "okay";
> +};
> +
> +&iiotimer1 {
> +	status = "okay";
> +};
> +
> +&mfd_timer3 {
> +	status = "okay";
> +};
> +
> +&pwm3 {
> +	pinctrl-0	= <&pwm3_pins>;
> +	pinctrl-names	= "default";
> +	status = "okay";
> +};
> +
> +&iiotimer3 {
> +	status = "okay";
> +};

I've always disliked this way of referencing nodes!

Any chance we can represent them in a hierarchy, so we don't lose that
information and we can get rid of all those horrible labels?

I'm happy to do the work.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

^ permalink raw reply

* Re: [PATCH v6 3/3] arm: dts: mt2701: Add node for Mediatek JPEG Decoder
From: Rick Chang @ 2016-11-23  9:43 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Hans Verkuil, Laurent Pinchart, Mauro Carvalho Chehab,
	Matthias Brugger, Rob Herring, linux-kernel, linux-media,
	srv_heupstream, linux-mediatek, linux-arm-kernel, devicetree,
	Minghsiu Tsai
In-Reply-To: <1479866054.8964.21.camel@mtksdaap41>

On Wed, 2016-11-23 at 09:54 +0800, Rick Chang wrote:
> Hi Hans,
> 
> On Tue, 2016-11-22 at 13:43 +0100, Hans Verkuil wrote:
> > On 22/11/16 04:21, Rick Chang wrote:
> > > Hi Hans,
> > >
> > > On Mon, 2016-11-21 at 15:51 +0100, Hans Verkuil wrote:
> > >> On 17/11/16 04:38, Rick Chang wrote:
> > >>> Signed-off-by: Rick Chang <rick.chang@mediatek.com>
> > >>> Signed-off-by: Minghsiu Tsai <minghsiu.tsai@mediatek.com>
> > >>> ---
> > >>> This patch depends on:
> > >>>   CCF "Add clock support for Mediatek MT2701"[1]
> > >>>   iommu and smi "Add the dtsi node of iommu and smi for mt2701"[2]
> > >>>
> > >>> [1] http://lists.infradead.org/pipermail/linux-mediatek/2016-October/007271.html
> > >>> [2] https://patchwork.kernel.org/patch/9164013/
> > >>
> > >> I assume that 1 & 2 will appear in 4.10? So this patch needs to go in
> > >> after the
> > >> other two are merged in 4.10?
> > >>
> > >> Regards,
> > >>
> > >> 	Hans
> > >
> > > [1] will appear in 4.10, but [2] will appear latter than 4.10.So this
> > > patch needs to go in after [1] & [2] will be merged in 4.11.
> > 
> > So what should I do? Merge the driver for 4.11 and wait with this patch
> > until [2] is merged in 4.11? Does that sound reasonable?
> > 
> > Regards,
> > 
> > 	Hans
> 
> What do you think about this? You merge the driver first and I send this
> patch again after [1] & [2] is merged.

BTW, to prevent merging conflict, the dtsi should be merged by mediatek
SoC maintainer, Matthias.I think we can only take care on the driver
part at this moment.

^ permalink raw reply

* Re: [PATCH v2 2/3] ARM: dts: sunxi: add support for Orange Pi Zero board
From: Andre Przywara @ 2016-11-23  9:23 UTC (permalink / raw)
  To: Maxime Ripard, Icenowy Zheng
  Cc: Mark Rutland, devicetree, Vishnu Patekar, Arnd Bergmann,
	Jonathan Corbet, linux-doc, Russell King, linux-kernel,
	Hans de Goede, Chen-Yu Tsai, linux-arm-kernel
In-Reply-To: <20161123075713.ks6gmud3rszjbdsh@lukather>

Hi Maxime,

On 23/11/16 07:57, Maxime Ripard wrote:
> On Tue, Nov 22, 2016 at 12:24:20AM +0800, Icenowy Zheng wrote:
>> Orange Pi Zero is a board that came with the new Allwinner H2+ SoC.
>>
>> Add a device tree file for it.
>>
>> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
>> ---
>> Changes since v2:
>> - Use generic pinconf binding instead of legacy allwinner pinctrl binding.
>> - removed uart3, which is not accessible on Orange Pi Zero.
>> - Removed sun8i-h2plus.dtsi and make Orange Pi Zero dts directly include
>>   sun8i-h3.dtsi.
>> - Removed allwinner,sun8i-h3 compatible.
>>
>>  arch/arm/boot/dts/Makefile                       |   1 +
>>  arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts | 137 +++++++++++++++++++++++
> 
> Ditto, h2-plus-orangepi-zero.
> 
>>  2 files changed, 138 insertions(+)
>>  create mode 100644 arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts
>>
>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>> index 802a10d..51a1dd7 100644
>> --- a/arch/arm/boot/dts/Makefile
>> +++ b/arch/arm/boot/dts/Makefile
>> @@ -834,6 +834,7 @@ dtb-$(CONFIG_MACH_SUN8I) += \
>>  	sun8i-a33-sinlinx-sina33.dtb \
>>  	sun8i-a83t-allwinner-h8homlet-v2.dtb \
>>  	sun8i-a83t-cubietruck-plus.dtb \
>> +	sun8i-h2plus-orangepi-zero.dtb \
>>  	sun8i-h3-bananapi-m2-plus.dtb \
>>  	sun8i-h3-nanopi-neo.dtb \
>>  	sun8i-h3-orangepi-2.dtb \
>> diff --git a/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts b/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts
>> new file mode 100644
>> index 0000000..b428e47
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/sun8i-h2plus-orangepi-zero.dts
>> @@ -0,0 +1,137 @@
>> +/*
>> + * Copyright (C) 2016 Icenowy Zheng <icenowy@aosc.xyz>
>> + *
>> + * Based on sun8i-h3-orangepi-one.dts, which is:
>> + *   Copyright (C) 2016 Hans de Goede <hdegoede@redhat.com>
>> + *
>> + * This file is dual-licensed: you can use it either under the terms
>> + * of the GPL or the X11 license, at your option. Note that this dual
>> + * licensing only applies to this file, and not this project as a
>> + * whole.
>> + *
>> + *  a) This file is free software; you can redistribute it and/or
>> + *     modify it under the terms of the GNU General Public License as
>> + *     published by the Free Software Foundation; either version 2 of the
>> + *     License, or (at your option) any later version.
>> + *
>> + *     This file is distributed in the hope that it will be useful,
>> + *     but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + *     GNU General Public License for more details.
>> + *
>> + * Or, alternatively,
>> + *
>> + *  b) Permission is hereby granted, free of charge, to any person
>> + *     obtaining a copy of this software and associated documentation
>> + *     files (the "Software"), to deal in the Software without
>> + *     restriction, including without limitation the rights to use,
>> + *     copy, modify, merge, publish, distribute, sublicense, and/or
>> + *     sell copies of the Software, and to permit persons to whom the
>> + *     Software is furnished to do so, subject to the following
>> + *     conditions:
>> + *
>> + *     The above copyright notice and this permission notice shall be
>> + *     included in all copies or substantial portions of the Software.
>> + *
>> + *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
>> + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>> + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
>> + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
>> + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>> + *     OTHER DEALINGS IN THE SOFTWARE.
>> + */
>> +
>> +/dts-v1/;
>> +#include "sun8i-h3.dtsi"
>> +#include "sunxi-common-regulators.dtsi"
>> +
>> +#include <dt-bindings/gpio/gpio.h>
>> +#include <dt-bindings/input/input.h>
>> +#include <dt-bindings/pinctrl/sun4i-a10.h>
>> +
>> +/ {
>> +	model = "Xunlong Orange Pi Zero";
>> +	compatible = "xunlong,orangepi-zero", "allwinner,sun8i-h2plus";
>> +
>> +	aliases {
>> +		serial0 = &uart0;
>> +	};
>> +
>> +	chosen {
>> +		stdout-path = "serial0:115200n8";
>> +	};
>> +
>> +	leds {
>> +		compatible = "gpio-leds";
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&leds_opi0>, <&leds_r_opi0>;
>> +
>> +		pwr_led {
>> +			label = "orangepi:green:pwr";
>> +			gpios = <&r_pio 0 10 GPIO_ACTIVE_HIGH>;
>> +			default-state = "on";
>> +		};
>> +
>> +		status_led {
>> +			label = "orangepi:red:status";
>> +			gpios = <&pio 0 17 GPIO_ACTIVE_HIGH>;
>> +		};
>> +	};
>> +};
>> +
>> +&ehci1 {
>> +	status = "okay";
>> +};
>> +
>> +&mmc0 {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&mmc0_pins_a>, <&mmc0_cd_pin>;
>> +	vmmc-supply = <&reg_vcc3v3>;
>> +	bus-width = <4>;
>> +	cd-gpios = <&pio 5 6 GPIO_ACTIVE_HIGH>; /* PF6 */
>> +	cd-inverted;
>> +	status = "okay";
>> +};
>> +
>> +&ohci1 {
>> +	status = "okay";
>> +};
>> +
>> +&pio {
>> +	leds_opi0: led_pins@0 {
>> +		pins = "PA17";
>> +		function = "gpio_out";
>> +	};
>> +};
>> +
>> +&r_pio {
>> +	leds_r_opi0: led_pins@0 {
>> +		pins = "PL10";
>> +		function = "gpio_out";
>> +	};
>> +};
>> +
>> +&uart0 {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&uart0_pins_a>;
>> +	status = "okay";
>> +};
>> +
>> +&uart1 {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&uart1_pins>;
>> +	status = "disabled";
>> +};
>> +
>> +&uart2 {
>> +	pinctrl-names = "default";
>> +	pinctrl-0 = <&uart2_pins>;
>> +	status = "disabled";
>> +};
> 
> I'm not sure you answered me on this one. Are those exposed on the
> headers? why did you put them as disabled here?

So they are on headers, though you have to solder the actual header pins
yourself [1]. But also these are the normal pins multiplexed with GPIOs
and other peripherals, so keeping them disabled is in line with the
existing policy, if I got this correctly.

I agree that the status="disabled" is redundant, since we have that
exact line already in the .dtsi. But I saw it in other DTs as well, most
prominently in the sun8i-h3-orangepi-one.dts.

So I think we should remove the "status=" lines here, dtc will generate
an identical dtb out of it. But we should keep the uart descriptions in
to make it easier for users to see which SoC pins are used for these
pins labeled UART[012] in the board description and schematic. Also all
it takes to enable those is to overwrite the status property, which can
easily be done inline (without resizing the dtb).

Cheers,
Andre.

[1] http://linux-sunxi.org/Xunlong_Orange_Pi_Zero

^ permalink raw reply

* Re: [PATCH 1/7] add binding for stm32 multifunctions timer driver
From: Lee Jones @ 2016-11-23  9:21 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: robh+dt, Mark Rutland, alexandre.torgue, devicetree,
	Linux Kernel Mailing List, Thierry Reding, linux-pwm, jic23,
	knaack.h, Lars-Peter Clausen, Peter Meerwald-Stadler, linux-iio,
	linux-arm-kernel, Fabrice Gasnier, Gerald Baeza, Arnaud Pouliquen,
	Linus Walleij, Linaro Kernel Mailman List, Benjamin Gaignard
In-Reply-To: <CA+M3ks4cVLtVe4zSvSiDUz6jKy0Wbw8j24VuStf_31D5ntwfvw@mail.gmail.com>

On Wed, 23 Nov 2016, Benjamin Gaignard wrote:

> 2016-11-22 17:52 GMT+01:00 Lee Jones <lee.jones@linaro.org>:
> > On Tue, 22 Nov 2016, Benjamin Gaignard wrote:
> >
> >> Add bindings information for stm32 timer MFD
> >>
> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> >> ---
> >>  .../devicetree/bindings/mfd/stm32-timer.txt        | 53 ++++++++++++++++++++++
> >>  1 file changed, 53 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/mfd/stm32-timer.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/mfd/stm32-timer.txt b/Documentation/devicetree/bindings/mfd/stm32-timer.txt
> >> new file mode 100644
> >> index 0000000..3cefce1
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/mfd/stm32-timer.txt
> >> @@ -0,0 +1,53 @@
> >> +STM32 multifunctions timer driver
> >
> > "STM32 Multi-Function Timer/PWM device bindings"
> >
> > Doesn't this shared device have a better name?
> 
> In SoC documentation those hardware blocks are named "advanced-control
> timers", "general purpose timers" or "basic timers"
> "stm32-timer" name is already used for clock source driver, that why I
> have prefix it with mfd

MFD is a Linuxisum and has no place in hardware description.

Please used one of the names you mentioned above.

Hopefully the one that best fits.

> >> +stm32 timer MFD allow to handle at the same time pwm and IIO timer devices
> >
> > No need for this sentence.
> >
> OK
> 
> >> +Required parameters:
> >> +- compatible: must be one of the follow value:
> >> +     "st,stm32-mfd-timer1"
> >> +     "st,stm32-mfd-timer2"
> >> +     "st,stm32-mfd-timer3"
> >> +     "st,stm32-mfd-timer4"
> >> +     "st,stm32-mfd-timer5"
> >> +     "st,stm32-mfd-timer6"
> >> +     "st,stm32-mfd-timer7"
> >> +     "st,stm32-mfd-timer8"
> >> +     "st,stm32-mfd-timer9"
> >> +     "st,stm32-mfd-timer10"
> >> +     "st,stm32-mfd-timer11"
> >> +     "st,stm32-mfd-timer12"
> >> +     "st,stm32-mfd-timer13"
> >> +     "st,stm32-mfd-timer14"
> >
> > We don't normally number devices.
> >
> > What's stopping you from simply doing:
> >
> >         pwm1: pwm1@40010000 {
> >                 compatible = "st,stm32-pwm";
> >         };
> >         pwm2: pwm1@40020000 {
> >                 compatible = "st,stm32-pwm";
> >         };
> >         pwm3: pwm1@40030000 {
> >                 compatible = "st,stm32-pwm";
> >         };
> >
> 
> Because each instance of the hardware is slightly different: number of
> pwm channels, triggers capabilities, etc ..
> so I need to distinguish them.
> Since it look to be a problem I will follow your suggestion and add a
> property this driver to be able to identify each instance.
> Do you think that "id" parameter (integer for 1 to 14) is acceptable ?

Unfortunately not. IDs aren't allowed in DT.

What about "pwm-chans" and "trigger"?

pwm-chans  	       : Number of available channels available
trigger		       : Boolean value specifying whether a timer is present

Why can't you let of_platform_populate() register the devices for you?
Then you can get rid of all of the meaningless numbers all over the place.

> >> +- reg :                      Physical base address and length of the controller's
> >> +                     registers.
> >> +- clock-names:               Set to "mfd_timer_clk".
> >
> Only one but I use devm_regmap_init_mmio_clk() to avoid calling
> clk_{enable/disable}
> everywhere in the drivers when reading/writing regsister.
> devm_regmap_init_mmio_clk() find the clock by it name that why I have
> put it here
> In the doc this clock in named "clk_int" I will use this name.

Please reply *below* the quote.

But okay, "clk_int" sounds like a more suitable name.

> > How many clocks are there?
> >
> > If only 1, you don't need this property.
> >
> > "mfd_timer_clk" is not the correct name.
> >
> > What is it called in the datasheet?
> >
> >> +- clocks:            Phandle of the clock used by the timer module.
> >
> > "Phandle to the clock ..."
> >
> >> +                     For Clk properties, please refer to [1].
> >> +- interrupts :               Reference to the timer interrupt
> >
> > Reference to?
> >
> > See how other binding documents describe this property.
> >
> >> +Optional parameters:
> >> +- resets :           Reference to a reset controller asserting the timer
> >
> > As above.
> >
> >> +Optional subnodes:
> >
> > Either use ":" or " :" or "<tab>:", but keep it consistent.
> >
> >> +- pwm:                       See Documentation/devicetree/bindings/pwm/pwm-stm32.txt
> >> +- iiotimer:          See Documentation/devicetree/bindings/iio/timer/stm32-iio-timer.txt
> >> +
> >> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> >
> > Use the relative paths "../clock/", "../pwm/", "../iio/".
> >
> OK
> 
> >> +Example:
> >> +     mfd_timer1: mfdtimer1@40010000 {
> >
> > This is not an "MFD timer".  MFD is a Linuxisum.
> >
> >> +             compatible = "st,stm32-mfd-timer1";
> >
> > Better description required.
> >
> >> +             reg = <0x40010000 0x400>;
> >> +             clocks = <&rcc 0 160>;
> >> +             clock-names = "mfd_timer_clk";
> >> +             interrupts = <27>;
> >> +
> >> +             pwm1: pwm1@40010000 {
> >> +                     compatible = "st,stm32-pwm1";
> >> +             };
> >> +
> >> +             iiotimer1: iiotimer1@40010000 {
> >> +                     compatible = "st,stm32-iio-timer1";
> >> +             };
> >> +     };

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* Re: [PATCH v2 3/3] arm64: dts: sunxi: enable EHCI1, OHCI1 and USB PHY nodes in Pine64
From: Maxime Ripard @ 2016-11-23  9:20 UTC (permalink / raw)
  To: Icenowy Zheng
  Cc: Mark Rutland, devicetree, Catalin Marinas, Will Deacon,
	linux-kernel, Chen-Yu Tsai, Rob Herring, linux-arm-kernel
In-Reply-To: <20161122155831.8724-3-icenowy@aosc.xyz>


[-- Attachment #1.1: Type: text/plain, Size: 476 bytes --]

On Tue, Nov 22, 2016 at 11:58:31PM +0800, Icenowy Zheng wrote:
> Pine64 have two USB Type-A ports, which are wired to the two ports of
> A64 USB PHY, and the lower port is the EHCI/OHCI1 port.
> 
> Enable the necessary nodes to enable the lower USB port to work.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>

Fixed the prefix and applied, thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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

^ permalink raw reply

* Re: [PATCH v2 2/3] arm64: dts: sunxi: sort the nodes in sun50i-a64-pine64.dts
From: Maxime Ripard @ 2016-11-23  9:19 UTC (permalink / raw)
  To: Icenowy Zheng
  Cc: Mark Rutland, devicetree, Catalin Marinas, Will Deacon,
	linux-kernel, Chen-Yu Tsai, Rob Herring, linux-arm-kernel
In-Reply-To: <20161122155831.8724-2-icenowy@aosc.xyz>


[-- Attachment #1.1: Type: text/plain, Size: 393 bytes --]

On Tue, Nov 22, 2016 at 11:58:30PM +0800, Icenowy Zheng wrote:
> In this dts file, uart0 node is put before i2c1.
> 
> Move the uart0 node to the end to satisfy alphebetical order.
> 
> Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>

Fixed the prefix and applied. Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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

^ permalink raw reply

* Re: [PATCH v2 3/3] ARM: dts: gose: add composite video input
From: Laurent Pinchart @ 2016-11-23  8:56 UTC (permalink / raw)
  To: Ulrich Hecht
  Cc: horms, linux-media, linux-renesas-soc, magnus.damm, hans.verkuil,
	niklas.soderlund, geert, sergei.shtylyov, devicetree
In-Reply-To: <2343930.U8AiEBNJBl@avalon>

Hello device tree maintainers,

On Tuesday 18 Oct 2016 18:50:39 Laurent Pinchart wrote:
> On Tuesday 18 Oct 2016 17:02:23 Ulrich Hecht wrote:
> > Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> > ---
> > 
> >  arch/arm/boot/dts/r8a7793-gose.dts | 36 +++++++++++++++++++++++++++++++
> >  1 file changed, 36 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/r8a7793-gose.dts
> > b/arch/arm/boot/dts/r8a7793-gose.dts index a47ea4b..2606021 100644
> > --- a/arch/arm/boot/dts/r8a7793-gose.dts
> > +++ b/arch/arm/boot/dts/r8a7793-gose.dts

[snip]

> > +	composite-in@20 {
> > +		compatible = "adi,adv7180";
> > +		reg = <0x20>;
> > +		remote = <&vin1>;
> > +
> > +		port {
> > +			adv7180: endpoint {
> > +				bus-width = <8>;
> > +				remote-endpoint = <&vin1ep>;
> > +			};
> > +		};
> 
> As explained before, you need to update the ADV7180 DT bindings first to
> document ports. I've discussed this with Hans last week, and we agreed that
> DT should model physical ports. Unfortunately the ADV7180 comes in four
> different packages with different feature sets that affect ports.
> 
> ADV7180  K CP32 Z               32-Lead Lead Frame Chip Scale Package
> ADV7180  B CP32 Z               32-Lead Lead Frame Chip Scale Package
> ADV7180 WB CP32 Z               32-Lead Lead Frame Chip Scale Package
> 
> ADV7180  B CP   Z               40-Lead Lead Frame Chip Scale Package
> ADV7180 WB CP   Z               40-Lead Lead Frame Chip Scale Package
> 
> ADV7180  K ST48 Z               48-Lead Low Profile Quad Flat Package
> ADV7180  B ST48 Z               48-Lead Low Profile Quad Flat Package
> ADV7180 WB ST48 Z               48-Lead Low Profile Quad Flat Package
> 
> ADV7180  B ST   Z               64-Lead Low Profile Quad Flat Package
> ADV7180 WB ST   Z               64-Lead Low Profile Quad Flat Package
> 
> W tells whether the part is qualified for automotive applications. It has no
> impact from a software point of view. K and B indicate the temperature
> range, and also have no software impact. The Z suffix indicates that the
> part is RoHS compliant (they all are) and also has no impact.
> 
> Unfortunately the W and K/B qualifiers come before the package qualifier.
> I'm not sure whether we could simply drop W, K/B and W and specify the
> following compatible strings
> 
> - adv7180cp32
> - adv7180cp
> - adv7180st48
> - adv7180st
> 
> or if we need more compatible strings that would match the full chip name.
> Feedback on that from the device tree maintainers would be appreciated.

Your input would be appreciated on this.

> Regardless of what compatible strings end up being used, the bindings should
> document 3 or 6 input ports depending on the model, and one output port.
> You can number the input ports from 0 to 2 or 0 to 5 depending on the model
> and the output port 3 or 6. Another option would be to number the output
> port 0 and the input ports 1 to 3 or 1 to 6 depending on the model. That
> would give a fixed number for the output port across all models, but might
> be a bit consuming as most bindings number input ports before output ports.
> 
> For the Gose board you should then add one composite connector to the device
> tree ("composite-video-connector") and connect it to port 0 of the
> ADV7180WBCP32.
> 
> > +	};
> > +

-- 
Regards,

Laurent Pinchart

^ permalink raw reply

* Re: [PATCH v4 3/4] drm/bridge: Add ti-tfp410 DVI transmitter driver
From: Jyri Sarha @ 2016-11-23  8:43 UTC (permalink / raw)
  To: dri-devel, devicetree
  Cc: khilman, bgolaszewski, tomi.valkeinen, laurent.pinchart
In-Reply-To: <2943f3cb3ea913686bc63e8496df63c82ce0f334.1479825908.git.jsarha@ti.com>

On 11/22/16 16:49, Jyri Sarha wrote:
> Add very basic ti-tfp410 DVI transmitter driver. The only feature
> separating this from a completely dummy bridge is the EDID read
> support trough DDC I2C. Even that functionality should be in a
> separate generic connector driver. However, because of missing DRM
> infrastructure support the connector is implemented within the bridge
> driver. Some tfp410 HW specific features may be added later if needed,
> because there is a set of registers behind i2c if it is connected.
> 
> This implementation is tested against my new tilcdc bridge support
> and it works with BeagleBone DVI-D Cape Rev A3. A DT binding document
> is also updated.
> 
> Signed-off-by: Jyri Sarha <jsarha@ti.com>
> ---
>  .../bindings/display/bridge/ti,tfp410.txt          |   9 +-
>  drivers/gpu/drm/bridge/Kconfig                     |   7 +
>  drivers/gpu/drm/bridge/Makefile                    |   1 +
>  drivers/gpu/drm/bridge/ti-tfp410.c                 | 311 +++++++++++++++++++++
>  4 files changed, 326 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/gpu/drm/bridge/ti-tfp410.c
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
> index 2cbe32a..54d7e31 100644
> --- a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
> @@ -6,10 +6,15 @@ Required properties:
>  
>  Optional properties:
>  - powerdown-gpios: power-down gpio
> +- reg: I2C address. If and only if present the device node
> +       should be placed into the i2c controller node where the
> +       tfp410 i2c is connected to.
>  
>  Required nodes:
> -- Video port 0 for DPI input
> -- Video port 1 for DVI output
> +- Video port 0 for DPI input [1].
> +- Video port 1 for DVI output [1].
> +
> +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
>  
>  Example
>  -------
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index bd6acc8..a424e03 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -81,6 +81,13 @@ config DRM_TOSHIBA_TC358767
>  	---help---
>  	  Toshiba TC358767 eDP bridge chip driver.
>  
> +config DRM_TI_TFP410
> +	tristate "TI TFP410 DVI/HDMI bridge"
> +	depends on OF
> +	select DRM_KMS_HELPER
> +	---help---
> +	  Texas Instruments TFP410 DVI/HDMI Transmitter driver
> +
>  source "drivers/gpu/drm/bridge/analogix/Kconfig"
>  
>  source "drivers/gpu/drm/bridge/adv7511/Kconfig"
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index 97ed1a5..8b065d9 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -11,3 +11,4 @@ obj-$(CONFIG_DRM_SII902X) += sii902x.o
>  obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
>  obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
>  obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
> +obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
> diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c
> new file mode 100644
> index 0000000..58e26cc
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/ti-tfp410.c
> @@ -0,0 +1,311 @@
> +/*
> + * Copyright (C) 2016 Texas Instruments
> + * Author: Jyri Sarha <jsarha@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/of_graph.h>
> +#include <linux/platform_device.h>
> +#include <linux/i2c.h>
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_crtc_helper.h>
> +
> +struct tfp410 {
> +	struct drm_bridge	bridge;
> +	struct drm_connector	connector;
> +
> +	struct i2c_adapter	*ddc;
> +
> +	struct device *dev;
> +};
> +
> +static inline struct tfp410 *
> +drm_bridge_to_tfp410(struct drm_bridge *bridge)
> +{
> +	return container_of(bridge, struct tfp410, bridge);
> +}
> +
> +static inline struct tfp410 *
> +drm_connector_to_tfp410(struct drm_connector *connector)
> +{
> +	return container_of(connector, struct tfp410, connector);
> +}
> +
> +static int tfp410_get_modes(struct drm_connector *connector)
> +{
> +	struct tfp410 *dvi = drm_connector_to_tfp410(connector);
> +	struct edid *edid;
> +	int ret;
> +
> +	if (!dvi->ddc)
> +		goto fallback;
> +
> +	edid = drm_get_edid(connector, dvi->ddc);
> +	if (!edid) {
> +		DRM_INFO("EDID read failed. Fallback to standard modes\n");
> +		goto fallback;
> +	}
> +
> +	drm_mode_connector_update_edid_property(connector, edid);
> +
> +	return drm_add_edid_modes(connector, edid);
> +fallback:
> +	/* No EDID, fallback on the XGA standard modes */
> +	ret = drm_add_modes_noedid(connector, 1920, 1200);
> +
> +	/* And prefer a mode pretty much anything can handle */
> +	drm_set_preferred_mode(connector, 1024, 768);
> +
> +	return ret;
> +}
> +
> +static const struct drm_connector_helper_funcs tfp410_con_helper_funcs = {
> +	.get_modes	= tfp410_get_modes,
> +};
> +
> +static enum drm_connector_status
> +tfp410_connector_detect(struct drm_connector *connector, bool force)
> +{
> +	struct tfp410 *dvi = drm_connector_to_tfp410(connector);
> +
> +	if (dvi->ddc) {
> +		if (drm_probe_ddc(dvi->ddc))
> +			return connector_status_connected;
> +		else
> +			return connector_status_disconnected;
> +	}
> +
> +	return connector_status_unknown;
> +}
> +
> +static const struct drm_connector_funcs tfp410_con_funcs = {
> +	.dpms			= drm_atomic_helper_connector_dpms,
> +	.detect			= tfp410_connector_detect,
> +	.fill_modes		= drm_helper_probe_single_connector_modes,
> +	.destroy		= drm_connector_cleanup,
> +	.reset			= drm_atomic_helper_connector_reset,
> +	.atomic_duplicate_state	= drm_atomic_helper_connector_duplicate_state,
> +	.atomic_destroy_state	= drm_atomic_helper_connector_destroy_state,
> +};
> +
> +static int tfp410_attach(struct drm_bridge *bridge)
> +{
> +	struct tfp410 *dvi = drm_bridge_to_tfp410(bridge);
> +	int ret;
> +
> +	if (!bridge->encoder) {
> +		dev_err(dvi->dev, "Missing encoder\n");
> +		return -ENODEV;
> +	}
> +
> +	drm_connector_helper_add(&dvi->connector,
> +				 &tfp410_con_helper_funcs);
> +	ret = drm_connector_init(bridge->dev, &dvi->connector,
> +				 &tfp410_con_funcs, DRM_MODE_CONNECTOR_HDMIA);
> +	if (ret) {
> +		dev_err(dvi->dev, "drm_connector_init() failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	drm_mode_connector_attach_encoder(&dvi->connector,
> +					  bridge->encoder);
> +
> +	return 0;
> +}
> +
> +static const struct drm_bridge_funcs tfp410_bridge_funcs = {
> +	.attach		= tfp410_attach,
> +};
> +
> +static int tfp410_get_connector_ddc(struct tfp410 *dvi)
> +{
> +	struct device_node *ep = NULL, *connector_node = NULL;
> +	struct device_node *ddc_phandle = NULL;
> +	int ret = 0;
> +
> +	/* port@1 is the connector node */
> +	ep = of_graph_get_endpoint_by_regs(dvi->dev->of_node, 1, -1);
> +	if (!ep)
> +		goto fail;
> +
> +	connector_node = of_graph_get_remote_port_parent(ep);
> +	if (!connector_node)
> +		goto fail;
> +
> +	ddc_phandle = of_parse_phandle(connector_node, "ddc-i2c-bus", 0);
> +	if (!ddc_phandle)
> +		goto fail;
> +
> +	dvi->ddc = of_get_i2c_adapter_by_node(ddc_phandle);
> +	if (dvi->ddc)
> +		dev_info(dvi->dev, "Connector's ddc i2c bus found\n");
> +	else
> +		ret = -EPROBE_DEFER;
> +
> +fail:
> +	of_node_put(ep);
> +	of_node_put(connector_node);
> +	of_node_put(ddc_phandle);
> +	return ret;
> +}
> +
> +static int tfp410_init(struct device *dev)
> +{
> +	struct tfp410 *dvi;
> +	int ret;
> +
> +	if (!dev->of_node) {
> +		dev_err(dev, "device-tree data is missing\n");
> +		return -ENXIO;
> +	}
> +
> +	dvi = devm_kzalloc(dev, sizeof(*dvi), GFP_KERNEL);
> +	if (!dvi)
> +		return -ENOMEM;
> +	dev_set_drvdata(dev, dvi);
> +
> +	dvi->bridge.funcs = &tfp410_bridge_funcs;
> +	dvi->bridge.of_node = dev->of_node;
> +	dvi->dev = dev;
> +
> +	ret = tfp410_get_connector_ddc(dvi);
> +	if (ret)
> +		goto fail;
> +
> +	ret = drm_bridge_add(&dvi->bridge);
> +	if (ret) {
> +		dev_err(dev, "drm_bridge_add() failed: %d\n", ret);
> +		goto fail;
> +	}
> +
> +	return 0;
> +fail:
> +	i2c_put_adapter(dvi->ddc);
> +	return ret;
> +}
> +
> +static int tfp410_fini(struct device *dev)
> +{
> +	struct tfp410 *dvi = dev_get_drvdata(dev);
> +
> +	drm_bridge_remove(&dvi->bridge);
> +
> +	if (dvi->ddc)
> +		i2c_put_adapter(dvi->ddc);
> +
> +	return 0;
> +}
> +
> +static int tfp410_probe(struct platform_device *pdev)
> +{
> +	return tfp410_init(&pdev->dev);
> +}
> +
> +static int tfp410_remove(struct platform_device *pdev)
> +{
> +	return tfp410_fini(&pdev->dev);
> +}
> +

Argh... I think I should still put all i2c code between
#if defined(CONFIG_I2C) || defined(CONFIG_I2C_MODULE)

#endif

Otherwise I think this is now finally ready.

> +/* There is currently no i2c functionality. */
> +static int tfp410_i2c_probe(struct i2c_client *client,
> +			    const struct i2c_device_id *id)
> +{
> +	int reg;
> +
> +	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;
> +	}
> +
> +	return tfp410_init(&client->dev);
> +}
> +
> +static int tfp410_i2c_remove(struct i2c_client *client)
> +{
> +	return tfp410_fini(&client->dev);
> +}
> +
> +static const struct of_device_id tfp410_match[] = {
> +	{ .compatible = "ti,tfp410" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, tfp410_match);
> +
> +struct platform_driver tfp410_platform_driver = {
> +	.probe	= tfp410_probe,
> +	.remove	= tfp410_remove,
> +	.driver	= {
> +		.name		= "tfp410-bridge",
> +		.of_match_table	= tfp410_match,
> +	},
> +};
> +
> +static const struct i2c_device_id tfp410_i2c_ids[] = {
> +	{ "tfp410", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, tfp410_i2c_ids);
> +
> +static struct i2c_driver tfp410_i2c_driver = {
> +	.driver = {
> +		.name	= "tfp410",
> +		.of_match_table = of_match_ptr(tfp410_match),
> +	},
> +	.id_table	= tfp410_i2c_ids,
> +	.probe		= tfp410_i2c_probe,
> +	.remove		= tfp410_i2c_remove,
> +};
> +
> +static struct {
> +	uint i2c:1;
> +	uint platform:1;
> +}  tfp410_registered_driver;
> +
> +static int __init tfp410_module_init(void)
> +{
> +	int ret;
> +
> +	ret = i2c_add_driver(&tfp410_i2c_driver);
> +	if (ret)
> +		pr_err("%s: registering i2c driver failed: %d",
> +		       __func__, ret);
> +	else
> +		tfp410_registered_driver.i2c = 1;
> +
> +	ret = platform_driver_register(&tfp410_platform_driver);
> +	if (ret)
> +		pr_err("%s: registering platform driver failed: %d",
> +		       __func__, ret);
> +	else
> +		tfp410_registered_driver.platform = 1;
> +
> +	if (tfp410_registered_driver.i2c ||
> +	    tfp410_registered_driver.platform)
> +		return 0;
> +
> +	return ret;
> +}
> +module_init(tfp410_module_init);
> +
> +static void __exit tfp410_module_exit(void)
> +{
> +	if (tfp410_registered_driver.i2c)
> +		i2c_del_driver(&tfp410_i2c_driver);
> +	if (tfp410_registered_driver.platform)
> +		platform_driver_unregister(&tfp410_platform_driver);
> +}
> +module_exit(tfp410_module_exit);
> +
> +MODULE_AUTHOR("Jyri Sarha <jsarha@ti.com>");
> +MODULE_DESCRIPTION("TI TFP410 DVI bridge driver");
> +MODULE_LICENSE("GPL");
> 

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

^ permalink raw reply

* [PATCH v17 4/4] soc: mediatek: Add Mediatek CMDQ helper
From: HS Liao @ 2016-11-23  8:39 UTC (permalink / raw)
  To: Rob Herring, Matthias Brugger, Jassi Brar
  Cc: Monica Wang, Jiaguang Zhang, Nicolas Boichat, cawa cheng, HS Liao,
	Bibby Hsieh, YT Shen, Damon Chu,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Sascha Hauer, Daoyuan Huang,
	Sascha Hauer, Glory Hung, CK HU,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	srv_heupstream-NuS5LvNUpcJWk0Htik3J/w, Josh-YC Liu,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Dennis-YC Hsieh,
	Philipp Zabel
In-Reply-To: <1479890343-4979-1-git-send-email-hs.liao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>

Add Mediatek CMDQ helper to create CMDQ packet and assemble GCE op code.

Signed-off-by: HS Liao <hs.liao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 drivers/soc/mediatek/Kconfig           |  11 ++
 drivers/soc/mediatek/Makefile          |   1 +
 drivers/soc/mediatek/mtk-cmdq-helper.c | 310 +++++++++++++++++++++++++++++++++
 include/linux/soc/mediatek/mtk-cmdq.h  | 174 ++++++++++++++++++
 4 files changed, 496 insertions(+)
 create mode 100644 drivers/soc/mediatek/mtk-cmdq-helper.c
 create mode 100644 include/linux/soc/mediatek/mtk-cmdq.h

diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
index 0a4ea80..94651ed 100644
--- a/drivers/soc/mediatek/Kconfig
+++ b/drivers/soc/mediatek/Kconfig
@@ -1,6 +1,17 @@
 #
 # MediaTek SoC drivers
 #
+config MTK_CMDQ
+	bool "MediaTek CMDQ Support"
+	depends on ARM64 && ( ARCH_MEDIATEK || COMPILE_TEST )
+	select MTK_CMDQ_MBOX
+	select MTK_INFRACFG
+	help
+	  Say yes here to add support for the MediaTek Command Queue (CMDQ)
+	  driver. The CMDQ is used to help read/write registers with critical
+	  time limitation, such as updating display configuration during the
+	  vblank.
+
 config MTK_INFRACFG
 	bool "MediaTek INFRACFG Support"
 	depends on ARCH_MEDIATEK || COMPILE_TEST
diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
index 12998b0..64ce5ee 100644
--- a/drivers/soc/mediatek/Makefile
+++ b/drivers/soc/mediatek/Makefile
@@ -1,3 +1,4 @@
+obj-$(CONFIG_MTK_CMDQ) += mtk-cmdq-helper.o
 obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o
 obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o
 obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c
new file mode 100644
index 0000000..7809e65
--- /dev/null
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -0,0 +1,310 @@
+/*
+ * Copyright (c) 2015 MediaTek Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/completion.h>
+#include <linux/errno.h>
+#include <linux/of_address.h>
+#include <linux/soc/mediatek/mtk-cmdq.h>
+
+#define CMDQ_SUBSYS_SHIFT	16
+#define CMDQ_ARG_A_WRITE_MASK	0xffff
+#define CMDQ_WRITE_ENABLE_MASK	BIT(0)
+#define CMDQ_EOC_IRQ_EN		BIT(0)
+#define CMDQ_EOC_CMD		((u64)((CMDQ_CODE_EOC << CMDQ_OP_CODE_SHIFT)) \
+				<< 32 | CMDQ_EOC_IRQ_EN)
+
+struct cmdq_subsys {
+	u32	base;
+	int	id;
+};
+
+static const struct cmdq_subsys gce_subsys[] = {
+	{0x1400, 1},
+	{0x1401, 2},
+	{0x1402, 3},
+};
+
+static int cmdq_subsys_base_to_id(u32 base)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(gce_subsys); i++)
+		if (gce_subsys[i].base == base)
+			return gce_subsys[i].id;
+	return -EFAULT;
+}
+
+static int cmdq_pkt_realloc_cmd_buffer(struct cmdq_pkt *pkt, size_t size)
+{
+	void *new_buf;
+
+	new_buf = krealloc(pkt->va_base, size, GFP_KERNEL | __GFP_ZERO);
+	if (!new_buf)
+		return -ENOMEM;
+	pkt->va_base = new_buf;
+	pkt->buf_size = size;
+	return 0;
+}
+
+struct cmdq_base *cmdq_register_device(struct device *dev)
+{
+	struct cmdq_base *cmdq_base;
+	struct resource res;
+	int subsys;
+	u32 base;
+
+	if (of_address_to_resource(dev->of_node, 0, &res))
+		return NULL;
+	base = (u32)res.start;
+
+	subsys = cmdq_subsys_base_to_id(base >> 16);
+	if (subsys < 0)
+		return NULL;
+
+	cmdq_base = devm_kmalloc(dev, sizeof(*cmdq_base), GFP_KERNEL);
+	if (!cmdq_base)
+		return NULL;
+	cmdq_base->subsys = subsys;
+	cmdq_base->base = base;
+
+	return cmdq_base;
+}
+EXPORT_SYMBOL(cmdq_register_device);
+
+struct cmdq_client *cmdq_mbox_create(struct device *dev, int index)
+{
+	struct cmdq_client *client;
+
+	client = kzalloc(sizeof(*client), GFP_KERNEL);
+	client->client.dev = dev;
+	client->client.tx_block = false;
+	client->chan = mbox_request_channel(&client->client, index);
+	return client;
+}
+EXPORT_SYMBOL(cmdq_mbox_create);
+
+void cmdq_mbox_destroy(struct cmdq_client *client)
+{
+	mbox_free_channel(client->chan);
+	kfree(client);
+}
+EXPORT_SYMBOL(cmdq_mbox_destroy);
+
+int cmdq_pkt_create(struct cmdq_pkt **pkt_ptr)
+{
+	struct cmdq_pkt *pkt;
+	int err;
+
+	pkt = kzalloc(sizeof(*pkt), GFP_KERNEL);
+	if (!pkt)
+		return -ENOMEM;
+	err = cmdq_pkt_realloc_cmd_buffer(pkt, PAGE_SIZE);
+	if (err < 0) {
+		kfree(pkt);
+		return err;
+	}
+	*pkt_ptr = pkt;
+	return 0;
+}
+EXPORT_SYMBOL(cmdq_pkt_create);
+
+void cmdq_pkt_destroy(struct cmdq_pkt *pkt)
+{
+	kfree(pkt->va_base);
+	kfree(pkt);
+}
+EXPORT_SYMBOL(cmdq_pkt_destroy);
+
+static bool cmdq_pkt_is_finalized(struct cmdq_pkt *pkt)
+{
+	u64 *expect_eoc;
+
+	if (pkt->cmd_buf_size < CMDQ_INST_SIZE << 1)
+		return false;
+
+	expect_eoc = pkt->va_base + pkt->cmd_buf_size - (CMDQ_INST_SIZE << 1);
+	if (*expect_eoc == CMDQ_EOC_CMD)
+		return true;
+
+	return false;
+}
+
+static int cmdq_pkt_append_command(struct cmdq_pkt *pkt, enum cmdq_code code,
+				   u32 arg_a, u32 arg_b)
+{
+	u64 *cmd_ptr;
+	int err;
+
+	if (WARN_ON(cmdq_pkt_is_finalized(pkt)))
+		return -EBUSY;
+	if (unlikely(pkt->cmd_buf_size + CMDQ_INST_SIZE > pkt->buf_size)) {
+		err = cmdq_pkt_realloc_cmd_buffer(pkt, pkt->buf_size << 1);
+		if (err < 0)
+			return err;
+	}
+	cmd_ptr = pkt->va_base + pkt->cmd_buf_size;
+	(*cmd_ptr) = (u64)((code << CMDQ_OP_CODE_SHIFT) | arg_a) << 32 | arg_b;
+	pkt->cmd_buf_size += CMDQ_INST_SIZE;
+	return 0;
+}
+
+int cmdq_pkt_write(struct cmdq_pkt *pkt, u32 value, struct cmdq_base *base,
+		   u32 offset)
+{
+	u32 arg_a = ((base->base + offset) & CMDQ_ARG_A_WRITE_MASK) |
+		    (base->subsys << CMDQ_SUBSYS_SHIFT);
+	return cmdq_pkt_append_command(pkt, CMDQ_CODE_WRITE, arg_a, value);
+}
+EXPORT_SYMBOL(cmdq_pkt_write);
+
+int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u32 value,
+			struct cmdq_base *base, u32 offset, u32 mask)
+{
+	u32 offset_mask = offset;
+	int err;
+
+	if (mask != 0xffffffff) {
+		err = cmdq_pkt_append_command(pkt, CMDQ_CODE_MASK, 0, ~mask);
+		if (err < 0)
+			return err;
+		offset_mask |= CMDQ_WRITE_ENABLE_MASK;
+	}
+	return cmdq_pkt_write(pkt, value, base, offset_mask);
+}
+EXPORT_SYMBOL(cmdq_pkt_write_mask);
+
+static const u32 cmdq_event_value[CMDQ_MAX_EVENT] = {
+	/* Display start of frame(SOF) events */
+	[CMDQ_EVENT_DISP_OVL0_SOF] = 11,
+	[CMDQ_EVENT_DISP_OVL1_SOF] = 12,
+	[CMDQ_EVENT_DISP_RDMA0_SOF] = 13,
+	[CMDQ_EVENT_DISP_RDMA1_SOF] = 14,
+	[CMDQ_EVENT_DISP_RDMA2_SOF] = 15,
+	[CMDQ_EVENT_DISP_WDMA0_SOF] = 16,
+	[CMDQ_EVENT_DISP_WDMA1_SOF] = 17,
+	/* Display end of frame(EOF) events */
+	[CMDQ_EVENT_DISP_OVL0_EOF] = 39,
+	[CMDQ_EVENT_DISP_OVL1_EOF] = 40,
+	[CMDQ_EVENT_DISP_RDMA0_EOF] = 41,
+	[CMDQ_EVENT_DISP_RDMA1_EOF] = 42,
+	[CMDQ_EVENT_DISP_RDMA2_EOF] = 43,
+	[CMDQ_EVENT_DISP_WDMA0_EOF] = 44,
+	[CMDQ_EVENT_DISP_WDMA1_EOF] = 45,
+	/* Mutex end of frame(EOF) events */
+	[CMDQ_EVENT_MUTEX0_STREAM_EOF] = 53,
+	[CMDQ_EVENT_MUTEX1_STREAM_EOF] = 54,
+	[CMDQ_EVENT_MUTEX2_STREAM_EOF] = 55,
+	[CMDQ_EVENT_MUTEX3_STREAM_EOF] = 56,
+	[CMDQ_EVENT_MUTEX4_STREAM_EOF] = 57,
+	/* Display underrun events */
+	[CMDQ_EVENT_DISP_RDMA0_UNDERRUN] = 63,
+	[CMDQ_EVENT_DISP_RDMA1_UNDERRUN] = 64,
+	[CMDQ_EVENT_DISP_RDMA2_UNDERRUN] = 65,
+};
+
+int cmdq_pkt_wfe(struct cmdq_pkt *pkt, enum cmdq_event event)
+{
+	u32 arg_b;
+
+	if (event >= CMDQ_MAX_EVENT || event < 0)
+		return -EINVAL;
+
+	/*
+	 * WFE arg_b
+	 * bit 0-11: wait value
+	 * bit 15: 1 - wait, 0 - no wait
+	 * bit 16-27: update value
+	 * bit 31: 1 - update, 0 - no update
+	 */
+	arg_b = CMDQ_WFE_UPDATE | CMDQ_WFE_WAIT | CMDQ_WFE_WAIT_VALUE;
+	return cmdq_pkt_append_command(pkt, CMDQ_CODE_WFE,
+			cmdq_event_value[event], arg_b);
+}
+EXPORT_SYMBOL(cmdq_pkt_wfe);
+
+int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, enum cmdq_event event)
+{
+	if (event >= CMDQ_MAX_EVENT || event < 0)
+		return -EINVAL;
+
+	return cmdq_pkt_append_command(pkt, CMDQ_CODE_WFE,
+			cmdq_event_value[event], CMDQ_WFE_UPDATE);
+}
+EXPORT_SYMBOL(cmdq_pkt_clear_event);
+
+static int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
+{
+	int err;
+
+	if (cmdq_pkt_is_finalized(pkt))
+		return 0;
+
+	/* insert EOC and generate IRQ for each command iteration */
+	err = cmdq_pkt_append_command(pkt, CMDQ_CODE_EOC, 0, CMDQ_EOC_IRQ_EN);
+	if (err < 0)
+		return err;
+
+	/* JUMP to end */
+	err = cmdq_pkt_append_command(pkt, CMDQ_CODE_JUMP, 0, CMDQ_JUMP_PASS);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
+int cmdq_pkt_flush_async(struct cmdq_client *client, struct cmdq_pkt *pkt,
+			 cmdq_async_flush_cb cb, void *data)
+{
+	int err;
+
+	err = cmdq_pkt_finalize(pkt);
+	if (err < 0)
+		return err;
+
+	pkt->cb.cb = cb;
+	pkt->cb.data = data;
+
+	mbox_send_message(client->chan, pkt);
+	/* We can send next packet immediately, so just call txdone. */
+	mbox_client_txdone(client->chan, 0);
+
+	return 0;
+}
+EXPORT_SYMBOL(cmdq_pkt_flush_async);
+
+struct cmdq_flush_completion {
+	struct completion cmplt;
+	bool err;
+};
+
+static void cmdq_pkt_flush_cb(struct cmdq_cb_data data)
+{
+	struct cmdq_flush_completion *cmplt = data.data;
+
+	cmplt->err = data.err;
+	complete(&cmplt->cmplt);
+}
+
+int cmdq_pkt_flush(struct cmdq_client *client, struct cmdq_pkt *pkt)
+{
+	struct cmdq_flush_completion cmplt;
+	int err;
+
+	init_completion(&cmplt.cmplt);
+	err = cmdq_pkt_flush_async(client, pkt, cmdq_pkt_flush_cb, &cmplt);
+	if (err < 0)
+		return err;
+	wait_for_completion(&cmplt.cmplt);
+	return cmplt.err ? -EFAULT : 0;
+}
+EXPORT_SYMBOL(cmdq_pkt_flush);
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h
new file mode 100644
index 0000000..5b35d73
--- /dev/null
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -0,0 +1,174 @@
+/*
+ * Copyright (c) 2015 MediaTek Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __MTK_CMDQ_H__
+#define __MTK_CMDQ_H__
+
+#include <linux/mailbox_client.h>
+#include <linux/mailbox/mtk-cmdq-mailbox.h>
+
+/* display events in command queue(CMDQ) */
+enum cmdq_event {
+	/* Display start of frame(SOF) events */
+	CMDQ_EVENT_DISP_OVL0_SOF,
+	CMDQ_EVENT_DISP_OVL1_SOF,
+	CMDQ_EVENT_DISP_RDMA0_SOF,
+	CMDQ_EVENT_DISP_RDMA1_SOF,
+	CMDQ_EVENT_DISP_RDMA2_SOF,
+	CMDQ_EVENT_DISP_WDMA0_SOF,
+	CMDQ_EVENT_DISP_WDMA1_SOF,
+	/* Display end of frame(EOF) events */
+	CMDQ_EVENT_DISP_OVL0_EOF,
+	CMDQ_EVENT_DISP_OVL1_EOF,
+	CMDQ_EVENT_DISP_RDMA0_EOF,
+	CMDQ_EVENT_DISP_RDMA1_EOF,
+	CMDQ_EVENT_DISP_RDMA2_EOF,
+	CMDQ_EVENT_DISP_WDMA0_EOF,
+	CMDQ_EVENT_DISP_WDMA1_EOF,
+	/* Mutex end of frame(EOF) events */
+	CMDQ_EVENT_MUTEX0_STREAM_EOF,
+	CMDQ_EVENT_MUTEX1_STREAM_EOF,
+	CMDQ_EVENT_MUTEX2_STREAM_EOF,
+	CMDQ_EVENT_MUTEX3_STREAM_EOF,
+	CMDQ_EVENT_MUTEX4_STREAM_EOF,
+	/* Display underrun events */
+	CMDQ_EVENT_DISP_RDMA0_UNDERRUN,
+	CMDQ_EVENT_DISP_RDMA1_UNDERRUN,
+	CMDQ_EVENT_DISP_RDMA2_UNDERRUN,
+	/* Keep this at the end */
+	CMDQ_MAX_EVENT,
+};
+
+struct cmdq_pkt;
+
+struct cmdq_base {
+	int	subsys;
+	u32	base;
+};
+
+struct cmdq_client {
+	struct mbox_client client;
+	struct mbox_chan *chan;
+};
+
+/**
+ * cmdq_register_device() - register device which needs CMDQ
+ * @dev:	device for CMDQ to access its registers
+ *
+ * Return: cmdq_base pointer or NULL for failed
+ */
+struct cmdq_base *cmdq_register_device(struct device *dev);
+
+/**
+ * cmdq_mbox_create() - create CMDQ mailbox client and channel
+ * @dev:	device of CMDQ mailbox client
+ * @index:	index of CMDQ mailbox channel
+ *
+ * Return: CMDQ mailbox client pointer
+ */
+struct cmdq_client *cmdq_mbox_create(struct device *dev, int index);
+
+/**
+ * cmdq_mbox_destroy() - destroy CMDQ mailbox client and channel
+ * @client:	the CMDQ mailbox client
+ */
+void cmdq_mbox_destroy(struct cmdq_client *client);
+
+/**
+ * cmdq_pkt_create() - create a CMDQ packet
+ * @pkt_ptr:	CMDQ packet pointer to retrieve cmdq_pkt
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_create(struct cmdq_pkt **pkt_ptr);
+
+/**
+ * cmdq_pkt_destroy() - destroy the CMDQ packet
+ * @pkt:	the CMDQ packet
+ */
+void cmdq_pkt_destroy(struct cmdq_pkt *pkt);
+
+/**
+ * cmdq_pkt_write() - append write command to the CMDQ packet
+ * @pkt:	the CMDQ packet
+ * @value:	the specified target register value
+ * @base:	the CMDQ base
+ * @offset:	register offset from module base
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_write(struct cmdq_pkt *pkt, u32 value,
+		   struct cmdq_base *base, u32 offset);
+
+/**
+ * cmdq_pkt_write_mask() - append write command with mask to the CMDQ packet
+ * @pkt:	the CMDQ packet
+ * @value:	the specified target register value
+ * @base:	the CMDQ base
+ * @offset:	register offset from module base
+ * @mask:	the specified target register mask
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u32 value,
+			struct cmdq_base *base, u32 offset, u32 mask);
+
+/**
+ * cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
+ * @pkt:	the CMDQ packet
+ * @event:	the desired event type to "wait and CLEAR"
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_wfe(struct cmdq_pkt *pkt, enum cmdq_event event);
+
+/**
+ * cmdq_pkt_clear_event() - append clear event command to the CMDQ packet
+ * @pkt:	the CMDQ packet
+ * @event:	the desired event to be cleared
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_clear_event(struct cmdq_pkt *pkt, enum cmdq_event event);
+
+/**
+ * cmdq_pkt_flush() - trigger CMDQ to execute the CMDQ packet
+ * @client:	the CMDQ mailbox client
+ * @pkt:	the CMDQ packet
+ *
+ * Return: 0 for success; else the error code is returned
+ *
+ * Trigger CMDQ to execute the CMDQ packet. Note that this is a
+ * synchronous flush function. When the function returned, the recorded
+ * commands have been done.
+ */
+int cmdq_pkt_flush(struct cmdq_client *client, struct cmdq_pkt *pkt);
+
+/**
+ * cmdq_pkt_flush_async() - trigger CMDQ to asynchronously execute the CMDQ
+ *                          packet and call back at the end of done packet
+ * @client:	the CMDQ mailbox client
+ * @pkt:	the CMDQ packet
+ * @cb:		called at the end of done packet
+ * @data:	this data will pass back to cb
+ *
+ * Return: 0 for success; else the error code is returned
+ *
+ * Trigger CMDQ to asynchronously execute the CMDQ packet and call back
+ * at the end of done packet. Note that this is an ASYNC function. When the
+ * function returned, it may or may not be finished.
+ */
+int cmdq_pkt_flush_async(struct cmdq_client *client, struct cmdq_pkt *pkt,
+			 cmdq_async_flush_cb cb, void *data);
+
+#endif	/* __MTK_CMDQ_H__ */
-- 
1.9.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox