Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1509636 > unrolled thread

[PATCH 0/2] ARM: da850: new drivers for better LCDC support

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/2] ARM: da850: new drivers for better LCDC support Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2016-10-26 19:40 +0200
    [PATCH 2/2] ARM: bus: da8xx-mstpri: new driver Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2016-10-26 19:40 +0200
      Re: [PATCH 2/2] ARM: bus: da8xx-mstpri: new driver Rob Herring <robh@kernel.org> - 2016-10-31 05:40 +0100
        Re: [PATCH 2/2] ARM: bus: da8xx-mstpri: new driver Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2016-10-31 10:50 +0100
          Re: [PATCH 2/2] ARM: bus: da8xx-mstpri: new driver Bartosz Golaszewski <bgolaszewski@baylibre.com> - 2016-10-31 11:00 +0100
            Re: [PATCH 2/2] ARM: bus: da8xx-mstpri: new driver Sekhar Nori <nsekhar@ti.com> - 2016-10-31 11:40 +0100
              Re: [PATCH 2/2] ARM: bus: da8xx-mstpri: new driver Kevin Hilman <khilman@baylibre.com> - 2016-10-31 19:30 +0100
          Re: [PATCH 2/2] ARM: bus: da8xx-mstpri: new driver Sekhar Nori <nsekhar@ti.com> - 2016-10-31 11:00 +0100

#1509636 — [PATCH 0/2] ARM: da850: new drivers for better LCDC support

This series adds two new drivers in order to better support the LCDC
rev1 present on the da850 boards.

The first patch adds a new memory driver which allows to write to the
DDR2/mDDR memory controller present on the da8xx SoCs. Since the
memory controller region is not mapped by anyone else, I went with
platform_get_resource() and ioremap() approach.

The second patch adds a new bus driver which allows to interact with
the MSTPRI registers of the SYSCFG0 module. The SYSCFG0 registers
are used by many drivers, hence the syscon/regmap approach.

As is mentioned in the comments: we don't want to commit to supporting
stable interfaces (DT bindings or sysfs attributes) so we hardcode the
settings required by some boards (for now only da850-lcdk) with the
hope that linux gets an appropriate framework for performance knobs
in the future.

Potential extensions of these drivers should be straightforward in the
future.

Tested on a da850-lcdk with a display connected over VGA and some
additional work on the tilcdc driver.

NOTE I'm sending this as v1, but it's a follow-up to a series I sent
previously and the RFC with the ddrctl driver. I dropped the dt patch
for now.

Bartosz Golaszewski (2):
  ARM: memory: da8xx-ddrctl: new driver
  ARM: bus: da8xx-mstpri: new driver

 .../devicetree/bindings/bus/ti,da850-mstpri.txt    |  20 ++
 .../memory-controllers/ti-da8xx-ddrctl.txt         |  20 ++
 drivers/bus/Kconfig                                |   9 +
 drivers/bus/Makefile                               |   2 +
 drivers/bus/da8xx-mstpri.c                         | 266 +++++++++++++++++++++
 drivers/memory/Kconfig                             |   8 +
 drivers/memory/Makefile                            |   1 +
 drivers/memory/da8xx-ddrctl.c                      | 175 ++++++++++++++
 8 files changed, 501 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/ti-da8xx-ddrctl.txt
 create mode 100644 drivers/bus/da8xx-mstpri.c
 create mode 100644 drivers/memory/da8xx-ddrctl.c

-- 
2.9.3

[toc] | [next] | [standalone]

#1509637 — [PATCH 2/2] ARM: bus: da8xx-mstpri: new driver

Create the driver for the da8xx master peripheral priority
configuration and implement support for writing to the three
Master Priority registers on da850 SoCs.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 .../devicetree/bindings/bus/ti,da850-mstpri.txt    |  20 ++
 drivers/bus/Kconfig                                |   9 +
 drivers/bus/Makefile                               |   2 +
 drivers/bus/da8xx-mstpri.c                         | 266 +++++++++++++++++++++
 4 files changed, 297 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt
 create mode 100644 drivers/bus/da8xx-mstpri.c

diff --git a/Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt b/Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt
new file mode 100644
index 0000000..225af09
--- /dev/null
+++ b/Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt
@@ -0,0 +1,20 @@
+* Device tree bindings for Texas Instruments da8xx master peripheral
+  priority driver
+
+DA8XX SoCs feature a set of registers allowing to change the priority of all
+peripherals classified as masters.
+
+Documentation:
+OMAP-L138 (DA850) - http://www.ti.com/lit/ug/spruh82c/spruh82c.pdf
+
+Required properties:
+
+- compatible:		"ti,da850-mstpri", "syscon" - for da850 based boards
+- reg:			offset and length of the mstpri registers
+
+Example for da850-lcdk is shown below.
+
+mstpri {
+	compatible = "ti,da850-mstpri";
+	reg = <0x14110 0x0c>;
+};
diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
index 7010dca..ed6a89c 100644
--- a/drivers/bus/Kconfig
+++ b/drivers/bus/Kconfig
@@ -166,4 +166,13 @@ config VEXPRESS_CONFIG
 	help
 	  Platform configuration infrastructure for the ARM Ltd.
 	  Versatile Express.
+
+config DA8XX_MSTPRI
+	bool "TI da8xx master peripheral priority driver"
+	depends on ARCH_DAVINCI_DA8XX
+	help
+	  Driver for Texas Instruments da8xx master peripheral priority
+	  configuration. Allows to adjust the priorities of all master
+	  peripherals.
+
 endmenu
diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
index c6cfa6b..2adb540 100644
--- a/drivers/bus/Makefile
+++ b/drivers/bus/Makefile
@@ -21,3 +21,5 @@ obj-$(CONFIG_SIMPLE_PM_BUS)	+= simple-pm-bus.o
 obj-$(CONFIG_TEGRA_ACONNECT)	+= tegra-aconnect.o
 obj-$(CONFIG_UNIPHIER_SYSTEM_BUS)	+= uniphier-system-bus.o
 obj-$(CONFIG_VEXPRESS_CONFIG)	+= vexpress-config.o
+
+obj-$(CONFIG_DA8XX_MSTPRI)	+= da8xx-mstpri.o
diff --git a/drivers/bus/da8xx-mstpri.c b/drivers/bus/da8xx-mstpri.c
new file mode 100644
index 0000000..00c4f83
--- /dev/null
+++ b/drivers/bus/da8xx-mstpri.c
@@ -0,0 +1,266 @@
+/*
+ * TI da8xx master peripheral priority driver
+ *
+ * Copyright (C) 2016 BayLibre SAS
+ *
+ * Author:
+ *   Bartosz Golaszewski <bgolaszewski@baylibre.com.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.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of_fdt.h>
+
+/*
+ * REVISIT: Linux doesn't have a good framework for the kind of performance
+ * knobs this driver controls. We can't use device tree properties as it deals
+ * with hardware configuration rather than description. We also don't want to
+ * commit to maintaining some random sysfs attributes.
+ *
+ * For now we just hardcode the register values for the boards that need
+ * some changes (as is the case for the LCD controller on da850-lcdk - the
+ * first board we support here). When linux gets an appropriate framework,
+ * we'll easily convert the driver to it.
+ */
+
+#define DA8XX_MSTPRI0_OFFSET		0
+#define DA8XX_MSTPRI1_OFFSET		4
+#define DA8XX_MSTPRI2_OFFSET		8
+
+enum {
+	DA8XX_MSTPRI_ARM_I = 0,
+	DA8XX_MSTPRI_ARM_D,
+	DA8XX_MSTPRI_UPP,
+	DA8XX_MSTPRI_SATA,
+	DA8XX_MSTPRI_PRU0,
+	DA8XX_MSTPRI_PRU1,
+	DA8XX_MSTPRI_EDMA30TC0,
+	DA8XX_MSTPRI_EDMA30TC1,
+	DA8XX_MSTPRI_EDMA31TC0,
+	DA8XX_MSTPRI_VPIF_DMA_0,
+	DA8XX_MSTPRI_VPIF_DMA_1,
+	DA8XX_MSTPRI_EMAC,
+	DA8XX_MSTPRI_USB0CFG,
+	DA8XX_MSTPRI_USB0CDMA,
+	DA8XX_MSTPRI_UHPI,
+	DA8XX_MSTPRI_USB1,
+	DA8XX_MSTPRI_LCDC,
+};
+
+struct da8xx_mstpri_descr {
+	int reg;
+	int shift;
+	int mask;
+};
+
+static const struct da8xx_mstpri_descr da8xx_mstpri_priority_list[] = {
+	[DA8XX_MSTPRI_ARM_I] = {
+		.reg = DA8XX_MSTPRI0_OFFSET,
+		.shift = 0,
+		.mask = 0x0000000f,
+	},
+	[DA8XX_MSTPRI_ARM_D] = {
+		.reg = DA8XX_MSTPRI0_OFFSET,
+		.shift = 4,
+		.mask = 0x000000f0,
+	},
+	[DA8XX_MSTPRI_UPP] = {
+		.reg = DA8XX_MSTPRI0_OFFSET,
+		.shift = 16,
+		.mask = 0x000f0000,
+	},
+	[DA8XX_MSTPRI_SATA] = {
+		.reg = DA8XX_MSTPRI0_OFFSET,
+		.shift = 20,
+		.mask = 0x00f00000,
+	},
+	[DA8XX_MSTPRI_PRU0] = {
+		.reg = DA8XX_MSTPRI1_OFFSET,
+		.shift = 0,
+		.mask = 0x0000000f,
+	},
+	[DA8XX_MSTPRI_PRU1] = {
+		.reg = DA8XX_MSTPRI1_OFFSET,
+		.shift = 4,
+		.mask = 0x000000f0,
+	},
+	[DA8XX_MSTPRI_EDMA30TC0] = {
+		.reg = DA8XX_MSTPRI1_OFFSET,
+		.shift = 8,
+		.mask = 0x00000f00,
+	},
+	[DA8XX_MSTPRI_EDMA30TC1] = {
+		.reg = DA8XX_MSTPRI1_OFFSET,
+		.shift = 12,
+		.mask = 0x0000f000,
+	},
+	[DA8XX_MSTPRI_EDMA31TC0] = {
+		.reg = DA8XX_MSTPRI1_OFFSET,
+		.shift = 16,
+		.mask = 0x000f0000,
+	},
+	[DA8XX_MSTPRI_VPIF_DMA_0] = {
+		.reg = DA8XX_MSTPRI1_OFFSET,
+		.shift = 24,
+		.mask = 0x0f000000,
+	},
+	[DA8XX_MSTPRI_VPIF_DMA_1] = {
+		.reg = DA8XX_MSTPRI1_OFFSET,
+		.shift = 28,
+		.mask = 0xf0000000,
+	},
+	[DA8XX_MSTPRI_EMAC] = {
+		.reg = DA8XX_MSTPRI2_OFFSET,
+		.shift = 0,
+		.mask = 0x0000000f,
+	},
+	[DA8XX_MSTPRI_USB0CFG] = {
+		.reg = DA8XX_MSTPRI2_OFFSET,
+		.shift = 8,
+		.mask = 0x00000f00,
+	},
+	[DA8XX_MSTPRI_USB0CDMA] = {
+		.reg = DA8XX_MSTPRI2_OFFSET,
+		.shift = 12,
+		.mask = 0x0000f000,
+	},
+	[DA8XX_MSTPRI_UHPI] = {
+		.reg = DA8XX_MSTPRI2_OFFSET,
+		.shift = 20,
+		.mask = 0x00f00000,
+	},
+	[DA8XX_MSTPRI_USB1] = {
+		.reg = DA8XX_MSTPRI2_OFFSET,
+		.shift = 24,
+		.mask = 0x0f000000,
+	},
+	[DA8XX_MSTPRI_LCDC] = {
+		.reg = DA8XX_MSTPRI2_OFFSET,
+		.shift = 28,
+		.mask = 0xf0000000,
+	},
+};
+
+struct da8xx_mstpri_priority {
+	int which;
+	u32 val;
+};
+
+struct da8xx_mstpri_board_priorities {
+	const char *board;
+	const struct da8xx_mstpri_priority *priorities;
+	size_t numprio;
+};
+
+/*
+ * Default memory settings of da850 do not meet the throughput/latency
+ * requirements of tilcdc. This results in the image displayed being
+ * incorrect and the following warning being displayed by the LCDC
+ * drm driver:
+ *
+ *   tilcdc da8xx_lcdc.0: tilcdc_crtc_irq(0x00000020): FIFO underfow
+ */
+static const struct da8xx_mstpri_priority da850_lcdk_priorities[] = {
+	{
+		.which = DA8XX_MSTPRI_LCDC,
+		.val = 0,
+	},
+	{
+		.which = DA8XX_MSTPRI_EDMA30TC1,
+		.val = 0,
+	},
+	{
+		.which = DA8XX_MSTPRI_EDMA30TC0,
+		.val = 1,
+	},
+};
+
+static const struct da8xx_mstpri_board_priorities da8xx_mstpri_board_confs[] = {
+	{
+		.board = "ti,da850-lcdk",
+		.priorities = da850_lcdk_priorities,
+		.numprio = ARRAY_SIZE(da850_lcdk_priorities),
+	},
+};
+
+static const struct da8xx_mstpri_board_priorities *
+da8xx_mstpri_get_board_prio(void)
+{
+	const struct da8xx_mstpri_board_priorities *board_prio;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(da8xx_mstpri_board_confs); i++) {
+		board_prio = &da8xx_mstpri_board_confs[i];
+
+		if (of_machine_is_compatible(board_prio->board))
+			return board_prio;
+	}
+
+	return NULL;
+}
+
+static int da8xx_mstpri_probe(struct platform_device *pdev)
+{
+	const struct da8xx_mstpri_board_priorities *prio_list;
+	const struct da8xx_mstpri_descr *prio_descr;
+	const struct da8xx_mstpri_priority *prio;
+	struct device *dev = &pdev->dev;
+	struct regmap *mstpri;
+	int i, ret;
+
+	mstpri = syscon_regmap_lookup_by_compatible("ti,da850-mstpri");
+	if (IS_ERR(mstpri)) {
+		dev_err(dev, "unable to map mstpri registers\n");
+		return PTR_ERR(mstpri);
+	}
+
+	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());
+		return -EINVAL;
+	}
+
+	for (i = 0; i < prio_list->numprio; i++) {
+		prio = &prio_list->priorities[i];
+		prio_descr = &da8xx_mstpri_priority_list[prio->which];
+
+		ret = regmap_update_bits(mstpri, prio_descr->reg,
+					 prio_descr->mask,
+					 prio->val << prio_descr->shift);
+		if (ret) {
+			dev_warn(dev,
+				 "error updating bits for register [%d]: %d\n",
+				 prio_descr->reg, ret);
+			continue;
+		}
+	}
+
+	return 0;
+}
+
+static const struct of_device_id da8xx_mstpri_of_match[] = {
+	{ .compatible = "ti,da850-mstpri", },
+	{ },
+};
+
+static struct platform_driver da8xx_mstpri_driver = {
+	.probe = da8xx_mstpri_probe,
+	.driver = {
+		.name = "da8xx-mstpri",
+		.of_match_table = da8xx_mstpri_of_match,
+	},
+};
+module_platform_driver(da8xx_mstpri_driver);
+
+MODULE_AUTHOR("Bartosz Golaszewski <bgolaszewski@baylibre.com>");
+MODULE_DESCRIPTION("TI da8xx master peripheral priority driver");
+MODULE_LICENSE("GPL v2");
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1512317 — Re: [PATCH 2/2] ARM: bus: da8xx-mstpri: new driver

On Wed, Oct 26, 2016 at 07:35:55PM +0200, Bartosz Golaszewski wrote:
> Create the driver for the da8xx master peripheral priority
> configuration and implement support for writing to the three
> Master Priority registers on da850 SoCs.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  .../devicetree/bindings/bus/ti,da850-mstpri.txt    |  20 ++
>  drivers/bus/Kconfig                                |   9 +
>  drivers/bus/Makefile                               |   2 +
>  drivers/bus/da8xx-mstpri.c                         | 266 +++++++++++++++++++++
>  4 files changed, 297 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt
>  create mode 100644 drivers/bus/da8xx-mstpri.c
> 
> diff --git a/Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt b/Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt
> new file mode 100644
> index 0000000..225af09
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt
> @@ -0,0 +1,20 @@
> +* Device tree bindings for Texas Instruments da8xx master peripheral
> +  priority driver
> +
> +DA8XX SoCs feature a set of registers allowing to change the priority of all
> +peripherals classified as masters.
> +
> +Documentation:
> +OMAP-L138 (DA850) - http://www.ti.com/lit/ug/spruh82c/spruh82c.pdf
> +
> +Required properties:
> +
> +- compatible:		"ti,da850-mstpri", "syscon" - for da850 based boards

Drop syscon. Doesn't look like it is needed and the example doesn't 
match.

> +- reg:			offset and length of the mstpri registers
> +
> +Example for da850-lcdk is shown below.
> +
> +mstpri {
> +	compatible = "ti,da850-mstpri";
> +	reg = <0x14110 0x0c>;
> +};

[toc] | [prev] | [next] | [standalone]

#1512431 — Re: [PATCH 2/2] ARM: bus: da8xx-mstpri: new driver

2016-10-31 5:30 GMT+01:00 Rob Herring <robh@kernel.org>:
> On Wed, Oct 26, 2016 at 07:35:55PM +0200, Bartosz Golaszewski wrote:
>> Create the driver for the da8xx master peripheral priority
>> configuration and implement support for writing to the three
>> Master Priority registers on da850 SoCs.
>>
>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>> ---
>>  .../devicetree/bindings/bus/ti,da850-mstpri.txt    |  20 ++
>>  drivers/bus/Kconfig                                |   9 +
>>  drivers/bus/Makefile                               |   2 +
>>  drivers/bus/da8xx-mstpri.c                         | 266 +++++++++++++++++++++
>>  4 files changed, 297 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt
>>  create mode 100644 drivers/bus/da8xx-mstpri.c
>>
>> diff --git a/Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt b/Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt
>> new file mode 100644
>> index 0000000..225af09
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt
>> @@ -0,0 +1,20 @@
>> +* Device tree bindings for Texas Instruments da8xx master peripheral
>> +  priority driver
>> +
>> +DA8XX SoCs feature a set of registers allowing to change the priority of all
>> +peripherals classified as masters.
>> +
>> +Documentation:
>> +OMAP-L138 (DA850) - http://www.ti.com/lit/ug/spruh82c/spruh82c.pdf
>> +
>> +Required properties:
>> +
>> +- compatible:                "ti,da850-mstpri", "syscon" - for da850 based boards
>
> Drop syscon. Doesn't look like it is needed and the example doesn't
> match.

Hi Rob,

it is needed: syscon_regmap_lookup_by_compatible() fails without it. I
fixed the example instead.

Thanks,
Bartosz Golaszewski

[toc] | [prev] | [next] | [standalone]

#1512437 — Re: [PATCH 2/2] ARM: bus: da8xx-mstpri: new driver

2016-10-31 10:52 GMT+01:00 Sekhar Nori <nsekhar@ti.com>:
> Hi Bartosz,
>
> On Monday 31 October 2016 03:10 PM, Bartosz Golaszewski wrote:
>> 2016-10-31 5:30 GMT+01:00 Rob Herring <robh@kernel.org>:
>>> On Wed, Oct 26, 2016 at 07:35:55PM +0200, Bartosz Golaszewski wrote:
>>>> Create the driver for the da8xx master peripheral priority
>>>> configuration and implement support for writing to the three
>>>> Master Priority registers on da850 SoCs.
>>>>
>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>>> ---
>>>>  .../devicetree/bindings/bus/ti,da850-mstpri.txt    |  20 ++
>>>>  drivers/bus/Kconfig                                |   9 +
>>>>  drivers/bus/Makefile                               |   2 +
>>>>  drivers/bus/da8xx-mstpri.c                         | 266 +++++++++++++++++++++
>>>>  4 files changed, 297 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt
>>>>  create mode 100644 drivers/bus/da8xx-mstpri.c
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt b/Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt
>>>> new file mode 100644
>>>> index 0000000..225af09
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt
>>>> @@ -0,0 +1,20 @@
>>>> +* Device tree bindings for Texas Instruments da8xx master peripheral
>>>> +  priority driver
>>>> +
>>>> +DA8XX SoCs feature a set of registers allowing to change the priority of all
>>>> +peripherals classified as masters.
>>>> +
>>>> +Documentation:
>>>> +OMAP-L138 (DA850) - http://www.ti.com/lit/ug/spruh82c/spruh82c.pdf
>>>> +
>>>> +Required properties:
>>>> +
>>>> +- compatible:                "ti,da850-mstpri", "syscon" - for da850 based boards
>>>
>>> Drop syscon. Doesn't look like it is needed and the example doesn't
>>> match.
>>
>> Hi Rob,
>>
>> it is needed: syscon_regmap_lookup_by_compatible() fails without it. I
>> fixed the example instead.
>
> Why are master priority registers under syscon? This driver should be
> the only entity touching them. So do we need an MFD driver?
>

It should, but syscfg0 registers are mapped all over the place. I
thought it would be safer to put them under syscon and Kevin agreed.

Thanks,
Bartosz Golaszewski

[toc] | [prev] | [next] | [standalone]

#1512464 — Re: [PATCH 2/2] ARM: bus: da8xx-mstpri: new driver

On Monday 31 October 2016 03:24 PM, Bartosz Golaszewski wrote:
> 2016-10-31 10:52 GMT+01:00 Sekhar Nori <nsekhar@ti.com>:
>> Hi Bartosz,
>>
>> On Monday 31 October 2016 03:10 PM, Bartosz Golaszewski wrote:
>>> 2016-10-31 5:30 GMT+01:00 Rob Herring <robh@kernel.org>:
>>>> On Wed, Oct 26, 2016 at 07:35:55PM +0200, Bartosz Golaszewski wrote:
>>>>> Create the driver for the da8xx master peripheral priority
>>>>> configuration and implement support for writing to the three
>>>>> Master Priority registers on da850 SoCs.
>>>>>
>>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>>>> ---
>>>>>  .../devicetree/bindings/bus/ti,da850-mstpri.txt    |  20 ++
>>>>>  drivers/bus/Kconfig                                |   9 +
>>>>>  drivers/bus/Makefile                               |   2 +
>>>>>  drivers/bus/da8xx-mstpri.c                         | 266 +++++++++++++++++++++
>>>>>  4 files changed, 297 insertions(+)
>>>>>  create mode 100644 Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt
>>>>>  create mode 100644 drivers/bus/da8xx-mstpri.c
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt b/Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt
>>>>> new file mode 100644
>>>>> index 0000000..225af09
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt
>>>>> @@ -0,0 +1,20 @@
>>>>> +* Device tree bindings for Texas Instruments da8xx master peripheral
>>>>> +  priority driver
>>>>> +
>>>>> +DA8XX SoCs feature a set of registers allowing to change the priority of all
>>>>> +peripherals classified as masters.
>>>>> +
>>>>> +Documentation:
>>>>> +OMAP-L138 (DA850) - http://www.ti.com/lit/ug/spruh82c/spruh82c.pdf
>>>>> +
>>>>> +Required properties:
>>>>> +
>>>>> +- compatible:                "ti,da850-mstpri", "syscon" - for da850 based boards
>>>>
>>>> Drop syscon. Doesn't look like it is needed and the example doesn't
>>>> match.
>>>
>>> Hi Rob,
>>>
>>> it is needed: syscon_regmap_lookup_by_compatible() fails without it. I
>>> fixed the example instead.
>>
>> Why are master priority registers under syscon? This driver should be
>> the only entity touching them. So do we need an MFD driver?
>>
> 
> It should, but syscfg0 registers are mapped all over the place. I

Not sure what you mean by this. Yes, the sysconfig module has many
functionalities. But the registers for a given functionality are all
grouped together AFAICS (except CHIPCFGn, see below).

Pinmux registers are part of syscfg module, but don't use syscon.

In the work going on for OHCI support, chipcfg registers are being put
under a syscon node. But that makes sense, I think, because chipcfg
registers are catering to really diverse functionality like PLL and EDMA
related stuff being placed in the same register - CHIPCFG0.

> thought it would be safer to put them under syscon and Kevin agreed.

Before using syscon for the whole syscfg space, I think we need some
analysis as to where the likely races are going to be.

Thanks,
Sekhar

[toc] | [prev] | [next] | [standalone]

#1512776 — Re: [PATCH 2/2] ARM: bus: da8xx-mstpri: new driver

Sekhar Nori <nsekhar@ti.com> writes:

> On Monday 31 October 2016 03:24 PM, Bartosz Golaszewski wrote:
>> 2016-10-31 10:52 GMT+01:00 Sekhar Nori <nsekhar@ti.com>:
>>> Hi Bartosz,
>>>
>>> On Monday 31 October 2016 03:10 PM, Bartosz Golaszewski wrote:
>>>> 2016-10-31 5:30 GMT+01:00 Rob Herring <robh@kernel.org>:
>>>>> On Wed, Oct 26, 2016 at 07:35:55PM +0200, Bartosz Golaszewski wrote:
>>>>>> Create the driver for the da8xx master peripheral priority
>>>>>> configuration and implement support for writing to the three
>>>>>> Master Priority registers on da850 SoCs.
>>>>>>
>>>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>>>>> ---
>>>>>>  .../devicetree/bindings/bus/ti,da850-mstpri.txt    |  20 ++
>>>>>>  drivers/bus/Kconfig                                |   9 +
>>>>>>  drivers/bus/Makefile                               |   2 +
>>>>>>  drivers/bus/da8xx-mstpri.c                         | 266 +++++++++++++++++++++
>>>>>>  4 files changed, 297 insertions(+)
>>>>>>  create mode 100644 Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt
>>>>>>  create mode 100644 drivers/bus/da8xx-mstpri.c
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt b/Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt
>>>>>> new file mode 100644
>>>>>> index 0000000..225af09
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt
>>>>>> @@ -0,0 +1,20 @@
>>>>>> +* Device tree bindings for Texas Instruments da8xx master peripheral
>>>>>> +  priority driver
>>>>>> +
>>>>>> +DA8XX SoCs feature a set of registers allowing to change the priority of all
>>>>>> +peripherals classified as masters.
>>>>>> +
>>>>>> +Documentation:
>>>>>> +OMAP-L138 (DA850) - http://www.ti.com/lit/ug/spruh82c/spruh82c.pdf
>>>>>> +
>>>>>> +Required properties:
>>>>>> +
>>>>>> +- compatible:                "ti,da850-mstpri", "syscon" - for da850 based boards
>>>>>
>>>>> Drop syscon. Doesn't look like it is needed and the example doesn't
>>>>> match.
>>>>
>>>> Hi Rob,
>>>>
>>>> it is needed: syscon_regmap_lookup_by_compatible() fails without it. I
>>>> fixed the example instead.
>>>
>>> Why are master priority registers under syscon? This driver should be
>>> the only entity touching them. So do we need an MFD driver?
>>>
>> 
>> It should, but syscfg0 registers are mapped all over the place. I
>
> Not sure what you mean by this. Yes, the sysconfig module has many
> functionalities. But the registers for a given functionality are all
> grouped together AFAICS (except CHIPCFGn, see below).
>
> Pinmux registers are part of syscfg module, but don't use syscon.
>
> In the work going on for OHCI support, chipcfg registers are being put
> under a syscon node. But that makes sense, I think, because chipcfg
> registers are catering to really diverse functionality like PLL and EDMA
> related stuff being placed in the same register - CHIPCFG0.
>
>> thought it would be safer to put them under syscon and Kevin agreed.
>
> Before using syscon for the whole syscfg space, I think we need some
> analysis as to where the likely races are going to be.

I'm OK with that for now.  It makes the most sense to use sycon only for
the register (ranges) that will actually be shared.

Kevin

[toc] | [prev] | [next] | [standalone]

#1512439 — Re: [PATCH 2/2] ARM: bus: da8xx-mstpri: new driver

Hi Bartosz,

On Monday 31 October 2016 03:10 PM, Bartosz Golaszewski wrote:
> 2016-10-31 5:30 GMT+01:00 Rob Herring <robh@kernel.org>:
>> On Wed, Oct 26, 2016 at 07:35:55PM +0200, Bartosz Golaszewski wrote:
>>> Create the driver for the da8xx master peripheral priority
>>> configuration and implement support for writing to the three
>>> Master Priority registers on da850 SoCs.
>>>
>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>> ---
>>>  .../devicetree/bindings/bus/ti,da850-mstpri.txt    |  20 ++
>>>  drivers/bus/Kconfig                                |   9 +
>>>  drivers/bus/Makefile                               |   2 +
>>>  drivers/bus/da8xx-mstpri.c                         | 266 +++++++++++++++++++++
>>>  4 files changed, 297 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt
>>>  create mode 100644 drivers/bus/da8xx-mstpri.c
>>>
>>> diff --git a/Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt b/Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt
>>> new file mode 100644
>>> index 0000000..225af09
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/bus/ti,da850-mstpri.txt
>>> @@ -0,0 +1,20 @@
>>> +* Device tree bindings for Texas Instruments da8xx master peripheral
>>> +  priority driver
>>> +
>>> +DA8XX SoCs feature a set of registers allowing to change the priority of all
>>> +peripherals classified as masters.
>>> +
>>> +Documentation:
>>> +OMAP-L138 (DA850) - http://www.ti.com/lit/ug/spruh82c/spruh82c.pdf
>>> +
>>> +Required properties:
>>> +
>>> +- compatible:                "ti,da850-mstpri", "syscon" - for da850 based boards
>>
>> Drop syscon. Doesn't look like it is needed and the example doesn't
>> match.
> 
> Hi Rob,
> 
> it is needed: syscon_regmap_lookup_by_compatible() fails without it. I
> fixed the example instead.

Why are master priority registers under syscon? This driver should be
the only entity touching them. So do we need an MFD driver?

Thanks,
Sekhar

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web