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


Groups > linux.kernel > #1349048 > unrolled thread

[PATCH 00/17] Add Initial support for PLX Technology OX810SE

Started byNeil Armstrong <narmstrong@baylibre.com>
First post2016-03-03 13:00 +0100
Last post2016-03-03 13:40 +0100
Articles 20 on this page of 21 — 6 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 00/17] Add Initial support for PLX Technology OX810SE Neil Armstrong <narmstrong@baylibre.com> - 2016-03-03 13:00 +0100
    [PATCH 06/17] reset: Add PLX Technology Reset Controller driver Neil Armstrong <narmstrong@baylibre.com> - 2016-03-03 13:00 +0100
      Re: [PATCH 06/17] reset: Add PLX Technology Reset Controller driver Philipp Zabel <p.zabel@pengutronix.de> - 2016-03-03 15:20 +0100
        Re: [PATCH 06/17] reset: Add PLX Technology Reset Controller driver Neil Armstrong <narmstrong@baylibre.com> - 2016-03-03 15:40 +0100
          Re: [PATCH 06/17] reset: Add PLX Technology Reset Controller driver Philipp Zabel <p.zabel@pengutronix.de> - 2016-03-03 16:10 +0100
    [PATCH 07/17] dt-bindings: Add PLX Technology Reset Controller bindings Neil Armstrong <narmstrong@baylibre.com> - 2016-03-03 13:00 +0100
      Re: [PATCH 07/17] dt-bindings: Add PLX Technology Reset Controller  bindings Philipp Zabel <p.zabel@pengutronix.de> - 2016-03-03 15:30 +0100
        Re: [PATCH 07/17] dt-bindings: Add PLX Technology Reset Controller  bindings Neil Armstrong <narmstrong@baylibre.com> - 2016-03-03 15:30 +0100
          Re: [PATCH 07/17] dt-bindings: Add PLX Technology Reset Controller  bindings Philipp Zabel <p.zabel@pengutronix.de> - 2016-03-03 15:40 +0100
    [PATCH 05/17] dt-bindings: Add PLX Technology RPS Timer bindings Neil Armstrong <narmstrong@baylibre.com> - 2016-03-03 13:00 +0100
    [PATCH 02/17] irqchip: Add PLX Technology RPS IRQ Controller Neil Armstrong <narmstrong@baylibre.com> - 2016-03-03 13:00 +0100
      Re: [PATCH 02/17] irqchip: Add PLX Technology RPS IRQ Controller Arnd Bergmann <arnd@arndb.de> - 2016-03-03 14:10 +0100
        Re: [PATCH 02/17] irqchip: Add PLX Technology RPS IRQ Controller Russell King - ARM Linux <linux@arm.linux.org.uk> - 2016-03-03 14:40 +0100
          Re: [PATCH 02/17] irqchip: Add PLX Technology RPS IRQ Controller Arnd Bergmann <arnd@arndb.de> - 2016-03-03 18:40 +0100
      Re: [PATCH 02/17] irqchip: Add PLX Technology RPS IRQ Controller Marc Zyngier <marc.zyngier@arm.com> - 2016-03-03 14:10 +0100
      RE: [PATCH 02/17] irqchip: Add PLX Technology RPS IRQ Controller "Ma Haijun" <mahaijuns@gmail.com> - 2016-03-03 16:40 +0100
        Re: [PATCH 02/17] irqchip: Add PLX Technology RPS IRQ Controller Neil Armstrong <narmstrong@baylibre.com> - 2016-03-03 18:00 +0100
          Re: [PATCH 02/17] irqchip: Add PLX Technology RPS IRQ Controller Marc Zyngier <marc.zyngier@arm.com> - 2016-03-03 18:20 +0100
    [PATCH 09/17] dt-bindings: Add PLX Technology OXNAS Standard Clocks bindings Neil Armstrong <narmstrong@baylibre.com> - 2016-03-03 13:00 +0100
    Re: [PATCH 00/17] Add Initial support for PLX Technology OX810SE Arnd Bergmann <arnd@arndb.de> - 2016-03-03 13:30 +0100
      Re: [PATCH 00/17] Add Initial support for PLX Technology OX810SE Neil Armstrong <narmstrong@baylibre.com> - 2016-03-03 13:40 +0100

Page 1 of 2  [1] 2  Next page →


#1349048 — [PATCH 00/17] Add Initial support for PLX Technology OX810SE

FromNeil Armstrong <narmstrong@baylibre.com>
Date2016-03-03 13:00 +0100
Subject[PATCH 00/17] Add Initial support for PLX Technology OX810SE
Message-ID<r8zVw-6xi-13@gated-at.bofh.it>
This serie adds initial support (IRQ, Timer, GPIO, Reset, Serial, Clocks) for
the PLX Technology OX810SE used in the well-known Western Digital My Book
World Edition Network Attached Storage device.

Extended support for SATA, DMA and Ethernet will come in further patches.

Upstream support for following devices like the OX820SE is welcome !

Neil Armstrong (17):
  dt-bindings: vendor-prefixes: Add PLX Technology
  irqchip: Add PLX Technology RPS IRQ Controller
  dt-bindings: Add PLX Technology RPS IRQ Controller bindings
  clocksource: Add PLX Technology RPS Timer
  dt-bindings: Add PLX Technology RPS Timer bindings
  reset: Add PLX Technology Reset Controller driver
  dt-bindings: Add PLX Technology Reset Controller bindings
  clk: Add PLX Technology OXNAS Standard Clocks
  dt-bindings: Add PLX Technology OXNAS Standard Clocks bindings
  pinctrl: Add PLX Technology OXNAS pinctrl and gpio driver
  dt-bindings: Add PLX Technology OXNAS pinctrl and gpio bindings
  arm: Add new mach-oxnas
  arm: Add build support for mach-oxnas
  arm: boot: dts: Add PLX Technology OX810SE dtsi
  dt-bindings: Add OXNAS bindings
  dt-bindings: Add Western Digital to vendor prefixes
  arm: boot: dts: Add Western Digital My Book World Edition device tree

 Documentation/devicetree/bindings/arm/oxnas.txt    |    9 +
 .../devicetree/bindings/clock/plxtech,stdclk.txt   |   24 +
 .../devicetree/bindings/gpio/gpio_oxnas.txt        |   27 +
 .../interrupt-controller/plxtech,rps-irq.txt       |   17 +
 .../bindings/pinctrl/plxtech,pinctrl.txt           |  100 ++
 .../devicetree/bindings/reset/plxtech,reset.txt    |   25 +
 .../bindings/timer/plxtech,rps-timer.txt           |   17 +
 .../devicetree/bindings/vendor-prefixes.txt        |    2 +
 arch/arm/Kconfig                                   |    2 +
 arch/arm/Makefile                                  |    1 +
 arch/arm/boot/dts/Makefile                         |    2 +
 arch/arm/boot/dts/ox810se.dtsi                     |  279 ++++
 arch/arm/boot/dts/wd-mbwe.dts                      |  106 ++
 arch/arm/mach-oxnas/Kconfig                        |   24 +
 arch/arm/mach-oxnas/Makefile                       |    1 +
 arch/arm/mach-oxnas/oxnas.c                        |   34 +
 drivers/clk/Kconfig                                |    6 +
 drivers/clk/Makefile                               |    1 +
 drivers/clk/clk-oxnas.c                            |  159 +++
 drivers/clocksource/Kconfig                        |    6 +
 drivers/clocksource/Makefile                       |    1 +
 drivers/clocksource/timer-rps.c                    |  249 ++++
 drivers/irqchip/Kconfig                            |    5 +
 drivers/irqchip/Makefile                           |    1 +
 drivers/irqchip/irq-rps.c                          |  128 ++
 drivers/pinctrl/Kconfig                            |    9 +
 drivers/pinctrl/Makefile                           |    1 +
 drivers/pinctrl/pinctrl-oxnas.c                    | 1393 ++++++++++++++++++++
 drivers/reset/Kconfig                              |    4 +
 drivers/reset/Makefile                             |    1 +
 drivers/reset/reset-oxnas.c                        |  149 +++
 31 files changed, 2783 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/oxnas.txt
 create mode 100644 Documentation/devicetree/bindings/clock/plxtech,stdclk.txt
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio_oxnas.txt
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/plxtech,rps-irq.txt
 create mode 100644 Documentation/devicetree/bindings/pinctrl/plxtech,pinctrl.txt
 create mode 100644 Documentation/devicetree/bindings/reset/plxtech,reset.txt
 create mode 100644 Documentation/devicetree/bindings/timer/plxtech,rps-timer.txt
 create mode 100644 arch/arm/boot/dts/ox810se.dtsi
 create mode 100644 arch/arm/boot/dts/wd-mbwe.dts
 create mode 100644 arch/arm/mach-oxnas/Kconfig
 create mode 100644 arch/arm/mach-oxnas/Makefile
 create mode 100644 arch/arm/mach-oxnas/oxnas.c
 create mode 100644 drivers/clk/clk-oxnas.c
 create mode 100644 drivers/clocksource/timer-rps.c
 create mode 100644 drivers/irqchip/irq-rps.c
 create mode 100644 drivers/pinctrl/pinctrl-oxnas.c
 create mode 100644 drivers/reset/reset-oxnas.c

-- 
1.9.1

[toc] | [next] | [standalone]


#1349049 — [PATCH 06/17] reset: Add PLX Technology Reset Controller driver

FromNeil Armstrong <narmstrong@baylibre.com>
Date2016-03-03 13:00 +0100
Subject[PATCH 06/17] reset: Add PLX Technology Reset Controller driver
Message-ID<r8A5c-6Bm-5@gated-at.bofh.it>
In reply to#1349048
Add System reset controller driver for PLX Technology OXNAS SoC Family.

CC: Ma Haijun <mahaijuns@gmail.com>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/reset/Kconfig       |   4 ++
 drivers/reset/Makefile      |   1 +
 drivers/reset/reset-oxnas.c | 149 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 154 insertions(+)
 create mode 100644 drivers/reset/reset-oxnas.c

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index df37212..f0ea63b 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -12,5 +12,9 @@ menuconfig RESET_CONTROLLER
 
 	  If unsure, say no.
 
+config RESET_OXNAS
+	bool
+	select MFD_SYSCON
+
 source "drivers/reset/sti/Kconfig"
 source "drivers/reset/hisilicon/Kconfig"
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
index 4d7178e..97e04c5 100644
--- a/drivers/reset/Makefile
+++ b/drivers/reset/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_ARCH_STI) += sti/
 obj-$(CONFIG_ARCH_HISI) += hisilicon/
 obj-$(CONFIG_ARCH_ZYNQ) += reset-zynq.o
 obj-$(CONFIG_ATH79) += reset-ath79.o
+obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
diff --git a/drivers/reset/reset-oxnas.c b/drivers/reset/reset-oxnas.c
new file mode 100644
index 0000000..d0ab670
--- /dev/null
+++ b/drivers/reset/reset-oxnas.c
@@ -0,0 +1,149 @@
+/*
+ * drivers/reset/reset-oxnas.c
+ *
+ * Copyright (C) 2016 Neil Armstrong <narmstrong@baylibre.com>
+ * Copyright (C) 2014 Ma Haijun <mahaijuns@gmail.com>
+ * Copyright (C) 2009 Oxford Semiconductor Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/reset-controller.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/types.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
+
+/* Regmap offsets */
+#define RST_SET_REGOFFSET	0x34
+#define RST_CLR_REGOFFSET	0x38
+
+struct oxnas_reset {
+	struct regmap *regmap;
+	struct reset_controller_dev rcdev;
+};
+
+static int oxnas_reset_reset(struct reset_controller_dev *rcdev,
+			      unsigned long id)
+{
+	struct oxnas_reset *data =
+		container_of(rcdev, struct oxnas_reset, rcdev);
+
+	regmap_write(data->regmap, RST_SET_REGOFFSET, BIT(id));
+	msleep(50);
+	regmap_write(data->regmap, RST_CLR_REGOFFSET, BIT(id));
+
+	return 0;
+}
+
+static int oxnas_reset_assert(struct reset_controller_dev *rcdev,
+			      unsigned long id)
+{
+	struct oxnas_reset *data =
+		container_of(rcdev, struct oxnas_reset, rcdev);
+
+	regmap_write(data->regmap, RST_SET_REGOFFSET, BIT(id));
+
+	return 0;
+}
+
+static int oxnas_reset_deassert(struct reset_controller_dev *rcdev,
+				unsigned long id)
+{
+	struct oxnas_reset *data =
+		container_of(rcdev, struct oxnas_reset, rcdev);
+
+	regmap_write(data->regmap, RST_CLR_REGOFFSET, BIT(id));
+
+	return 0;
+}
+
+static struct reset_control_ops oxnas_reset_ops = {
+	.reset		= oxnas_reset_reset,
+	.assert		= oxnas_reset_assert,
+	.deassert	= oxnas_reset_deassert,
+};
+
+static const struct of_device_id oxnas_reset_dt_ids[] = {
+	 { .compatible = "plxtech,nas782x-reset", },
+	 { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, oxnas_reset_dt_ids);
+
+static int oxnas_reset_probe(struct platform_device *pdev)
+{
+	struct oxnas_reset *data;
+	struct device *parent;
+
+	parent = pdev->dev.parent;
+	if (!parent) {
+		dev_err(&pdev->dev, "no parent\n");
+		return -ENODEV;
+	}
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->regmap = syscon_node_to_regmap(parent->of_node);
+	if (IS_ERR(data->regmap)) {
+		dev_err(&pdev->dev, "failed to get parent regmap\n");
+		return -ENODEV;
+	}
+
+	data->rcdev.owner = THIS_MODULE;
+	data->rcdev.nr_resets = 32;
+	data->rcdev.ops = &oxnas_reset_ops;
+	data->rcdev.of_node = pdev->dev.of_node;
+	reset_controller_register(&data->rcdev);
+
+	platform_set_drvdata(pdev, data);
+
+	return 0;
+}
+
+static int oxnas_reset_remove(struct platform_device *pdev)
+{
+	struct oxnas_reset *data = platform_get_drvdata(pdev);
+
+	reset_controller_unregister(&data->rcdev);
+
+	return 0;
+}
+
+static struct platform_driver oxnas_reset_driver = {
+	.probe	= oxnas_reset_probe,
+	.remove	= oxnas_reset_remove,
+	.driver = {
+		.name		= "oxnas-reset",
+		.owner		= THIS_MODULE,
+		.of_match_table	= oxnas_reset_dt_ids,
+	},
+};
+
+static int __init oxnas_reset_init(void)
+{
+	return platform_driver_probe(&oxnas_reset_driver,
+				     oxnas_reset_probe);
+}
+
+/*
+ * Reset controller does not support probe deferral, so it has to be
+ * initialized before any user, in particular, PCIE uses subsys_initcall.
+ */
+arch_initcall(oxnas_reset_init);
-- 
1.9.1

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


#1349192 — Re: [PATCH 06/17] reset: Add PLX Technology Reset Controller driver

FromPhilipp Zabel <p.zabel@pengutronix.de>
Date2016-03-03 15:20 +0100
SubjectRe: [PATCH 06/17] reset: Add PLX Technology Reset Controller driver
Message-ID<r8CgF-8cy-1@gated-at.bofh.it>
In reply to#1349049
Hi Neil,

Am Donnerstag, den 03.03.2016, 12:39 +0100 schrieb Neil Armstrong:
> Add System reset controller driver for PLX Technology OXNAS SoC Family.
> 
> CC: Ma Haijun <mahaijuns@gmail.com>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/reset/Kconfig       |   4 ++
>  drivers/reset/Makefile      |   1 +
>  drivers/reset/reset-oxnas.c | 149 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 154 insertions(+)
>  create mode 100644 drivers/reset/reset-oxnas.c
> 
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index df37212..f0ea63b 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -12,5 +12,9 @@ menuconfig RESET_CONTROLLER
>  
>  	  If unsure, say no.
>  
> +config RESET_OXNAS
> +	bool
> +	select MFD_SYSCON

I'd prefer not to select MFD_SYSCON here, but rather let ARCH_OXNAS do
that.

>  source "drivers/reset/sti/Kconfig"
>  source "drivers/reset/hisilicon/Kconfig"
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 4d7178e..97e04c5 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -7,3 +7,4 @@ obj-$(CONFIG_ARCH_STI) += sti/
>  obj-$(CONFIG_ARCH_HISI) += hisilicon/
>  obj-$(CONFIG_ARCH_ZYNQ) += reset-zynq.o
>  obj-$(CONFIG_ATH79) += reset-ath79.o
> +obj-$(CONFIG_RESET_OXNAS) += reset-oxnas.o
> diff --git a/drivers/reset/reset-oxnas.c b/drivers/reset/reset-oxnas.c
> new file mode 100644
> index 0000000..d0ab670
> --- /dev/null
> +++ b/drivers/reset/reset-oxnas.c
> @@ -0,0 +1,149 @@
> +/*
> + * drivers/reset/reset-oxnas.c
> + *
> + * Copyright (C) 2016 Neil Armstrong <narmstrong@baylibre.com>
> + * Copyright (C) 2014 Ma Haijun <mahaijuns@gmail.com>
> + * Copyright (C) 2009 Oxford Semiconductor Ltd
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +#include <linux/err.h>
> +#include <linux/io.h>

Is there any need to include linux/io.h ?

> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +#include <linux/types.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
> +
> +/* Regmap offsets */
> +#define RST_SET_REGOFFSET	0x34
> +#define RST_CLR_REGOFFSET	0x38
> +
> +struct oxnas_reset {
> +	struct regmap *regmap;
> +	struct reset_controller_dev rcdev;
> +};
> +
> +static int oxnas_reset_reset(struct reset_controller_dev *rcdev,
> +			      unsigned long id)
> +{
> +	struct oxnas_reset *data =
> +		container_of(rcdev, struct oxnas_reset, rcdev);
> +
> +	regmap_write(data->regmap, RST_SET_REGOFFSET, BIT(id));
> +	msleep(50);

Is this the right delay for all of the resets in this register?
If not, I'd drop the .reset callback.

> +	regmap_write(data->regmap, RST_CLR_REGOFFSET, BIT(id));
> +
> +	return 0;
> +}
> +
> +static int oxnas_reset_assert(struct reset_controller_dev *rcdev,
> +			      unsigned long id)
> +{
> +	struct oxnas_reset *data =
> +		container_of(rcdev, struct oxnas_reset, rcdev);
> +
> +	regmap_write(data->regmap, RST_SET_REGOFFSET, BIT(id));
> +
> +	return 0;
> +}
> +
> +static int oxnas_reset_deassert(struct reset_controller_dev *rcdev,
> +				unsigned long id)
> +{
> +	struct oxnas_reset *data =
> +		container_of(rcdev, struct oxnas_reset, rcdev);
> +
> +	regmap_write(data->regmap, RST_CLR_REGOFFSET, BIT(id));
> +
> +	return 0;
> +}
> +
> +static struct reset_control_ops oxnas_reset_ops = {

const

> +	.reset		= oxnas_reset_reset,
> +	.assert		= oxnas_reset_assert,
> +	.deassert	= oxnas_reset_deassert,
> +};
> +
> +static const struct of_device_id oxnas_reset_dt_ids[] = {
> +	 { .compatible = "plxtech,nas782x-reset", },
> +	 { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, oxnas_reset_dt_ids);
> +
> +static int oxnas_reset_probe(struct platform_device *pdev)
> +{
> +	struct oxnas_reset *data;
> +	struct device *parent;
> +
> +	parent = pdev->dev.parent;
> +	if (!parent) {
> +		dev_err(&pdev->dev, "no parent\n");

Can this even happen?

> +		return -ENODEV;
> +	}
> +
> +	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	data->regmap = syscon_node_to_regmap(parent->of_node);
> +	if (IS_ERR(data->regmap)) {
> +		dev_err(&pdev->dev, "failed to get parent regmap\n");
> +		return -ENODEV;

Better print the error code and return it.

> +	}
> +
> +	data->rcdev.owner = THIS_MODULE;
> +	data->rcdev.nr_resets = 32;
> +	data->rcdev.ops = &oxnas_reset_ops;
> +	data->rcdev.of_node = pdev->dev.of_node;
> +	reset_controller_register(&data->rcdev);

Move this down a bit:

> +
> +	platform_set_drvdata(pdev, data);
> +
> +	return 0;

and
	return reset_controller_register(&data->rcdev);
here.

> +}
> +
> +static int oxnas_reset_remove(struct platform_device *pdev)
> +{
> +	struct oxnas_reset *data = platform_get_drvdata(pdev);
> +
> +	reset_controller_unregister(&data->rcdev);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver oxnas_reset_driver = {
> +	.probe	= oxnas_reset_probe,
> +	.remove	= oxnas_reset_remove,
> +	.driver = {
> +		.name		= "oxnas-reset",
> +		.owner		= THIS_MODULE,

The .owner field is overwritten by __platform_driver_register() anyway,
just drop it.

> +		.of_match_table	= oxnas_reset_dt_ids,
> +	},
> +};
> +
> +static int __init oxnas_reset_init(void)
> +{
> +	return platform_driver_probe(&oxnas_reset_driver,
> +				     oxnas_reset_probe);
> +}
> +
> +/*
> + * Reset controller does not support probe deferral, so it has to be
> + * initialized before any user, in particular, PCIE uses subsys_initcall.
> + */
> +arch_initcall(oxnas_reset_init);

That doesn't sound right. (of_)reset_control_get return -EPROBE_DEFER if
the rcdev isn't found in the list. Could you elaborate on this?

regards
Philipp

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


#1349234 — Re: [PATCH 06/17] reset: Add PLX Technology Reset Controller driver

FromNeil Armstrong <narmstrong@baylibre.com>
Date2016-03-03 15:40 +0100
SubjectRe: [PATCH 06/17] reset: Add PLX Technology Reset Controller driver
Message-ID<r8CA3-8lg-41@gated-at.bofh.it>
In reply to#1349192
On 03/03/2016 03:18 PM, Philipp Zabel wrote:
> Hi Neil,
> 
>> +config RESET_OXNAS
>> +	bool
>> +	select MFD_SYSCON
> 
> I'd prefer not to select MFD_SYSCON here, but rather let ARCH_OXNAS do
> that.
> 
OK.

>> +#include <linux/io.h>
> 
> Is there any need to include linux/io.h ?

No, dropping.

>> +static int oxnas_reset_reset(struct reset_controller_dev *rcdev,
>> +			      unsigned long id)
>> +{
>> +	struct oxnas_reset *data =
>> +		container_of(rcdev, struct oxnas_reset, rcdev);
>> +
>> +	regmap_write(data->regmap, RST_SET_REGOFFSET, BIT(id));
>> +	msleep(50);
> 
> Is this the right delay for all of the resets in this register?
> If not, I'd drop the .reset callback.
> 
The delay is not strictly necessary, but better to avoid any HW issues.
And the .reset callback is needed since reset_control_reset
does not assert -> deassert as fallback.

>> +	regmap_write(data->regmap, RST_CLR_REGOFFSET, BIT(id));
>> +
>> +	return 0;
>> +}
>> +
>> +static int oxnas_reset_assert(struct reset_controller_dev *rcdev,
>> +			      unsigned long id)
>> +{
>> +	struct oxnas_reset *data =
>> +		container_of(rcdev, struct oxnas_reset, rcdev);
>> +
>> +	regmap_write(data->regmap, RST_SET_REGOFFSET, BIT(id));
>> +
>> +	return 0;
>> +}
>> +
>> +static int oxnas_reset_deassert(struct reset_controller_dev *rcdev,
>> +				unsigned long id)
>> +{
>> +	struct oxnas_reset *data =
>> +		container_of(rcdev, struct oxnas_reset, rcdev);
>> +
>> +	regmap_write(data->regmap, RST_CLR_REGOFFSET, BIT(id));
>> +
>> +	return 0;
>> +}
>> +
>> +static struct reset_control_ops oxnas_reset_ops = {
> 
> const
> 
Something checkpatch should report...

>> +	.reset		= oxnas_reset_reset,
>> +	.assert		= oxnas_reset_assert,
>> +	.deassert	= oxnas_reset_deassert,
>> +};
>> +
>> +static const struct of_device_id oxnas_reset_dt_ids[] = {
>> +	 { .compatible = "plxtech,nas782x-reset", },
>> +	 { /* sentinel */ },
>> +};
>> +MODULE_DEVICE_TABLE(of, oxnas_reset_dt_ids);
>> +
>> +static int oxnas_reset_probe(struct platform_device *pdev)
>> +{
>> +	struct oxnas_reset *data;
>> +	struct device *parent;
>> +
>> +	parent = pdev->dev.parent;
>> +	if (!parent) {
>> +		dev_err(&pdev->dev, "no parent\n");
> 
> Can this even happen?
> 
It's to make sure parent->of_node is valid for syscon_node_to_regmap.

>> +		return -ENODEV;
>> +	}
>> +
>> +	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
>> +	if (!data)
>> +		return -ENOMEM;
>> +
>> +	data->regmap = syscon_node_to_regmap(parent->of_node);
>> +	if (IS_ERR(data->regmap)) {
>> +		dev_err(&pdev->dev, "failed to get parent regmap\n");
>> +		return -ENODEV;
> 
> Better print the error code and return it.
> 
Good point.

>> +	}
>> +
>> +	data->rcdev.owner = THIS_MODULE;
>> +	data->rcdev.nr_resets = 32;
>> +	data->rcdev.ops = &oxnas_reset_ops;
>> +	data->rcdev.of_node = pdev->dev.of_node;
>> +	reset_controller_register(&data->rcdev);
> 
> Move this down a bit:
> 
>> +
>> +	platform_set_drvdata(pdev, data);
>> +
>> +	return 0;
> 
> and
> 	return reset_controller_register(&data->rcdev);
> here.
> 
Yes, sound better...

>> +static struct platform_driver oxnas_reset_driver = {
>> +	.probe	= oxnas_reset_probe,
>> +	.remove	= oxnas_reset_remove,
>> +	.driver = {
>> +		.name		= "oxnas-reset",
>> +		.owner		= THIS_MODULE,
> 
> The .owner field is overwritten by __platform_driver_register() anyway,
> just drop it.
OK

>> +/*
>> + * Reset controller does not support probe deferral, so it has to be
>> + * initialized before any user, in particular, PCIE uses subsys_initcall.
>> + */
>> +arch_initcall(oxnas_reset_init);
> 
> That doesn't sound right. (of_)reset_control_get return -EPROBE_DEFER if
> the rcdev isn't found in the list. Could you elaborate on this?
It was an old change, I will put back the generic module platform init.

> 
> regards
> Philipp
> 
Thanks,

Neil

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


#1349270 — Re: [PATCH 06/17] reset: Add PLX Technology Reset Controller driver

FromPhilipp Zabel <p.zabel@pengutronix.de>
Date2016-03-03 16:10 +0100
SubjectRe: [PATCH 06/17] reset: Add PLX Technology Reset Controller driver
Message-ID<r8D35-k8-39@gated-at.bofh.it>
In reply to#1349234
Am Donnerstag, den 03.03.2016, 15:29 +0100 schrieb Neil Armstrong:
> >> +static int oxnas_reset_reset(struct reset_controller_dev *rcdev,
> >> +			      unsigned long id)
> >> +{
> >> +	struct oxnas_reset *data =
> >> +		container_of(rcdev, struct oxnas_reset, rcdev);
> >> +
> >> +	regmap_write(data->regmap, RST_SET_REGOFFSET, BIT(id));
> >> +	msleep(50);
> > 
> > Is this the right delay for all of the resets in this register?
> > If not, I'd drop the .reset callback.
> > 
> The delay is not strictly necessary, but better to avoid any HW issues.

Ok, maybe add a comment.

> And the .reset callback is needed since reset_control_reset
> does not assert -> deassert as fallback.

That's because some controllers don't even have manual
assertion/deassertion, and for some reset lines the drivers better know
the timing or they want to do other stuff while the reset is asserted.

[...]
> >> +static struct reset_control_ops oxnas_reset_ops = {
> > 
> > const
> > 
> Something checkpatch should report...

This is new in any case. rcdev->ops was not const* until recently.

> >> +	.reset		= oxnas_reset_reset,
> >> +	.assert		= oxnas_reset_assert,
> >> +	.deassert	= oxnas_reset_deassert,
> >> +};
> >> +
> >> +static const struct of_device_id oxnas_reset_dt_ids[] = {
> >> +	 { .compatible = "plxtech,nas782x-reset", },
> >> +	 { /* sentinel */ },
> >> +};
> >> +MODULE_DEVICE_TABLE(of, oxnas_reset_dt_ids);
> >> +
> >> +static int oxnas_reset_probe(struct platform_device *pdev)
> >> +{
> >> +	struct oxnas_reset *data;
> >> +	struct device *parent;
> >> +
> >> +	parent = pdev->dev.parent;
> >> +	if (!parent) {
> >> +		dev_err(&pdev->dev, "no parent\n");
> > 
> > Can this even happen?
> > 
> It's to make sure parent->of_node is valid for syscon_node_to_regmap.

Since this is a platform device probed via device tree,
pdev->dev.parent should always be set (see of_device_alloc()).

regards
Philipp

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


#1349061 — [PATCH 07/17] dt-bindings: Add PLX Technology Reset Controller bindings

FromNeil Armstrong <narmstrong@baylibre.com>
Date2016-03-03 13:00 +0100
Subject[PATCH 07/17] dt-bindings: Add PLX Technology Reset Controller bindings
Message-ID<r8A5d-6Bm-39@gated-at.bofh.it>
In reply to#1349048
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 .../devicetree/bindings/reset/plxtech,reset.txt    | 25 ++++++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reset/plxtech,reset.txt

diff --git a/Documentation/devicetree/bindings/reset/plxtech,reset.txt b/Documentation/devicetree/bindings/reset/plxtech,reset.txt
new file mode 100644
index 0000000..e99648d
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/plxtech,reset.txt
@@ -0,0 +1,25 @@
+PLX Technology OXNAS SoC Family RESET Controller
+================================================
+
+Please also refer to reset.txt in this directory for common reset
+controller binding usage.
+
+Required properties:
+- compatible: Should be "plxtech,nas782x-reset"
+- #reset-cells: 1, see below
+
+Parent node should have the following properties :
+- compatible: Should be "plxtech,ox810se-sys-ctrl", "syscon", "simple-mfd"
+
+example:
+
+sys: sys-ctrl@000000 {
+	compatible = "plxtech,ox810se-sys-ctrl", "syscon", "simple-mfd";
+	reg = <0x000000 0x100000>;
+
+	reset: reset-controller {
+		compatible = "plxtech,nas782x-reset";
+		#reset-cells = <1>;
+
+	};
+};
-- 
1.9.1

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


#1349207 — Re: [PATCH 07/17] dt-bindings: Add PLX Technology Reset Controller bindings

FromPhilipp Zabel <p.zabel@pengutronix.de>
Date2016-03-03 15:30 +0100
SubjectRe: [PATCH 07/17] dt-bindings: Add PLX Technology Reset Controller bindings
Message-ID<r8Cqm-8hu-9@gated-at.bofh.it>
In reply to#1349061
Am Donnerstag, den 03.03.2016, 12:40 +0100 schrieb Neil Armstrong:
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  .../devicetree/bindings/reset/plxtech,reset.txt    | 25 ++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/plxtech,reset.txt
> 
> diff --git a/Documentation/devicetree/bindings/reset/plxtech,reset.txt b/Documentation/devicetree/bindings/reset/plxtech,reset.txt
> new file mode 100644
> index 0000000..e99648d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/plxtech,reset.txt
> @@ -0,0 +1,25 @@
> +PLX Technology OXNAS SoC Family RESET Controller
> +================================================
> +
> +Please also refer to reset.txt in this directory for common reset
> +controller binding usage.
> +
> +Required properties:
> +- compatible: Should be "plxtech,nas782x-reset"
> +- #reset-cells: 1, see below
> +
> +Parent node should have the following properties :
> +- compatible: Should be "plxtech,ox810se-sys-ctrl", "syscon", "simple-mfd"
> +
> +example:
> +
> +sys: sys-ctrl@000000 {
> +	compatible = "plxtech,ox810se-sys-ctrl", "syscon", "simple-mfd";
> +	reg = <0x000000 0x100000>;
> +
> +	reset: reset-controller {
> +		compatible = "plxtech,nas782x-reset";
> +		#reset-cells = <1>;
> +
> +	};
> +};

Is there a list of the reset bits in this register?

regards
Philipp

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


#1349215 — Re: [PATCH 07/17] dt-bindings: Add PLX Technology Reset Controller bindings

FromNeil Armstrong <narmstrong@baylibre.com>
Date2016-03-03 15:30 +0100
SubjectRe: [PATCH 07/17] dt-bindings: Add PLX Technology Reset Controller bindings
Message-ID<r8Cqn-8hu-25@gated-at.bofh.it>
In reply to#1349207
On 03/03/2016 03:21 PM, Philipp Zabel wrote:
> Am Donnerstag, den 03.03.2016, 12:40 +0100 schrieb Neil Armstrong:
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> 
> Is there a list of the reset bits in this register?
> 
> regards
> Philipp
> 

Yes, should I add it to the bindings ?

Neil

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


#1349229 — Re: [PATCH 07/17] dt-bindings: Add PLX Technology Reset Controller bindings

FromPhilipp Zabel <p.zabel@pengutronix.de>
Date2016-03-03 15:40 +0100
SubjectRe: [PATCH 07/17] dt-bindings: Add PLX Technology Reset Controller bindings
Message-ID<r8CA2-8lg-23@gated-at.bofh.it>
In reply to#1349215
Am Donnerstag, den 03.03.2016, 15:24 +0100 schrieb Neil Armstrong:
> On 03/03/2016 03:21 PM, Philipp Zabel wrote:
> > Am Donnerstag, den 03.03.2016, 12:40 +0100 schrieb Neil Armstrong:
> >> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> > 
> > Is there a list of the reset bits in this register?
> > 
> > regards
> > Philipp
> > 
> 
> Yes, should I add it to the bindings ?

Yes, please. Either that or add a header file with #defines to
include/dt-bindings/reset and use it in the dtsi.

regards
Philipp

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


#1349065 — [PATCH 05/17] dt-bindings: Add PLX Technology RPS Timer bindings

FromNeil Armstrong <narmstrong@baylibre.com>
Date2016-03-03 13:00 +0100
Subject[PATCH 05/17] dt-bindings: Add PLX Technology RPS Timer bindings
Message-ID<r8A5e-6Bm-51@gated-at.bofh.it>
In reply to#1349048
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 .../devicetree/bindings/timer/plxtech,rps-timer.txt     | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/plxtech,rps-timer.txt

diff --git a/Documentation/devicetree/bindings/timer/plxtech,rps-timer.txt b/Documentation/devicetree/bindings/timer/plxtech,rps-timer.txt
new file mode 100644
index 0000000..bc7cb33
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/plxtech,rps-timer.txt
@@ -0,0 +1,17 @@
+PLX Technology OXNAS SoCs Family RPS Timer
+==========================================
+
+Required properties:
+- compatible: Should be "plxtech,nas782x-rps-timer"
+- reg : Specifies base physical address and size of the registers.
+- interrupts : The interrupt of the first timer
+- clocks : The phandle of the timer clock source
+
+example:
+
+timer0: timer@200 {
+	compatible = "plxtech,nas782x-rps-timer";
+	reg = <0x200 0x40>;
+	clocks = <&rpsclk>;
+	interrupts = <4 5>;
+};
-- 
1.9.1

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


#1349067 — [PATCH 02/17] irqchip: Add PLX Technology RPS IRQ Controller

FromNeil Armstrong <narmstrong@baylibre.com>
Date2016-03-03 13:00 +0100
Subject[PATCH 02/17] irqchip: Add PLX Technology RPS IRQ Controller
Message-ID<r8A5e-6Bm-55@gated-at.bofh.it>
In reply to#1349048
Add PLX Technology RPS IRQ Controller as irqchip driver.

CC: Ma Haijun <mahaijuns@gmail.com>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/irqchip/Kconfig   |   5 ++
 drivers/irqchip/Makefile  |   1 +
 drivers/irqchip/irq-rps.c | 128 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 134 insertions(+)
 create mode 100644 drivers/irqchip/irq-rps.c

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index fb50911..7892c1a 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -135,6 +135,11 @@ config PIC32_EVIC
 	select GENERIC_IRQ_CHIP
 	select IRQ_DOMAIN
 
+config PLXTECH_RPS
+	bool
+	select GENERIC_IRQ_CHIP
+	select IRQ_DOMAIN
+
 config RENESAS_INTC_IRQPIN
 	bool
 	select IRQ_DOMAIN
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 18caacb..3eec3a0 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_I8259)			+= irq-i8259.o
 obj-$(CONFIG_IMGPDC_IRQ)		+= irq-imgpdc.o
 obj-$(CONFIG_IRQ_MIPS_CPU)		+= irq-mips-cpu.o
 obj-$(CONFIG_SIRF_IRQ)			+= irq-sirfsoc.o
+obj-$(CONFIG_PLXTECH_RPS)		+= irq-rps.o
 obj-$(CONFIG_RENESAS_INTC_IRQPIN)	+= irq-renesas-intc-irqpin.o
 obj-$(CONFIG_RENESAS_IRQC)		+= irq-renesas-irqc.o
 obj-$(CONFIG_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o
diff --git a/drivers/irqchip/irq-rps.c b/drivers/irqchip/irq-rps.c
new file mode 100644
index 0000000..bcd4a31
--- /dev/null
+++ b/drivers/irqchip/irq-rps.c
@@ -0,0 +1,128 @@
+/*
+ * drivers/irqchip/irq-rps.c
+ *
+ * Copyright (C) 2009 Oxford Semiconductor Ltd
+ * Copyright (C) 2013 Ma Haijun <mahaijuns@gmail.com>
+ * Copyright (C) 2016 Neil Armstrong <narmstrong@baylibre.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/irqdomain.h>
+#include <linux/irq.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/version.h>
+#include <linux/irqchip.h>
+
+#include <asm/exception.h>
+
+struct rps_chip_data {
+	void __iomem *base;
+	struct irq_domain *domain;
+} rps_data;
+
+enum {
+	RPS_IRQ_COUNT = 32,
+
+	RPS_STATUS = 0,
+	RPS_RAW_STATUS = 4,
+	RPS_UNMASK = 8,
+	RPS_MASK = 0xc,
+};
+
+/* Routines to acknowledge, disable and enable interrupts */
+static void rps_mask_irq(struct irq_data *d)
+{
+	u32 mask = BIT(d->hwirq);
+
+	iowrite32(mask, rps_data.base + RPS_MASK);
+}
+
+static void rps_unmask_irq(struct irq_data *d)
+{
+	u32 mask = BIT(d->hwirq);
+
+	iowrite32(mask, rps_data.base + RPS_UNMASK);
+}
+
+static void rps_ack_irq(struct irq_data *d)
+{
+	/* NOP */
+}
+
+static void __exception_irq_entry handle_irq(struct pt_regs *regs)
+{
+	u32 irqstat;
+	int hwirq;
+
+	irqstat = ioread32(rps_data.base + RPS_STATUS);
+	hwirq = __ffs(irqstat);
+
+	do {
+		handle_IRQ(irq_find_mapping(rps_data.domain, hwirq), regs);
+
+		irqstat = ioread32(rps_data.base + RPS_STATUS);
+		hwirq = __ffs(irqstat);
+	} while (irqstat);
+}
+
+int __init rps_of_init(struct device_node *node, struct device_node *parent)
+{
+	int ret;
+	struct irq_chip_generic *gc;
+
+	if (WARN_ON(!node))
+		return -ENODEV;
+
+	rps_data.base = of_iomap(node, 0);
+	WARN(!rps_data.base, "unable to map rps registers\n");
+
+	rps_data.domain = irq_domain_add_linear(node, RPS_IRQ_COUNT,
+						&irq_generic_chip_ops,
+						NULL);
+	if (!rps_data.domain) {
+		pr_err("%s: could add irq domain\n",
+		       node->full_name);
+		return -ENOMEM;
+	}
+
+	ret = irq_alloc_domain_generic_chips(rps_data.domain, RPS_IRQ_COUNT, 1,
+					     "RPS", handle_level_irq,
+					     0, 0, IRQ_GC_INIT_NESTED_LOCK);
+	if (ret) {
+		pr_err("%s: could not allocate generic chip\n",
+		       node->full_name);
+		irq_domain_remove(rps_data.domain);
+		return -EINVAL;
+	}
+
+	gc = irq_get_domain_generic_chip(rps_data.domain, 0);
+	gc->chip_types[0].chip.irq_ack = rps_ack_irq;
+	gc->chip_types[0].chip.irq_mask = rps_mask_irq;
+	gc->chip_types[0].chip.irq_unmask = rps_unmask_irq;
+
+	/* Disable all IRQs */
+	iowrite32(~0, rps_data.base + RPS_MASK);
+
+	set_handle_irq(handle_irq);
+
+	pr_info("Registered %d rps interrupts\n", RPS_IRQ_COUNT);
+
+	return 0;
+}
+
+IRQCHIP_DECLARE(nas782x, "plxtech,nas782x-rps", rps_of_init);
-- 
1.9.1

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


#1349139 — Re: [PATCH 02/17] irqchip: Add PLX Technology RPS IRQ Controller

FromArnd Bergmann <arnd@arndb.de>
Date2016-03-03 14:10 +0100
SubjectRe: [PATCH 02/17] irqchip: Add PLX Technology RPS IRQ Controller
Message-ID<r8BaX-7xs-23@gated-at.bofh.it>
In reply to#1349067
On Thursday 03 March 2016 13:01:13 Marc Zyngier wrote:
> > +/* Routines to acknowledge, disable and enable interrupts */
> > +static void rps_mask_irq(struct irq_data *d)
> > +{
> > +     u32 mask = BIT(d->hwirq);
> > +
> > +     iowrite32(mask, rps_data.base + RPS_MASK);
> 
> I do question the use of iowrite32 here (and its ioread32 pendent
> anywhere else), as it actually translates in a writel, which contains a
> memory barrier. Do you have any case that requires the use of such a
> barrier? if not, consider switching to relaxed accessors (which are the
> 

I really ask everyone to do the opposite: we have seen several drivers
blindlessly using the relaxed accessors and actually introducing bugs
that way, so I'd rather see the readl/writel ones used by default.

In any performance critical code, it's reasonable to take a closer
look and use the relaxed version with an added comment explaining
why it's safe there.

	Arnd

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


#1349159 — Re: [PATCH 02/17] irqchip: Add PLX Technology RPS IRQ Controller

FromRussell King - ARM Linux <linux@arm.linux.org.uk>
Date2016-03-03 14:40 +0100
SubjectRe: [PATCH 02/17] irqchip: Add PLX Technology RPS IRQ Controller
Message-ID<r8BDX-7Jt-7@gated-at.bofh.it>
In reply to#1349139
On Thu, Mar 03, 2016 at 02:08:19PM +0100, Arnd Bergmann wrote:
> On Thursday 03 March 2016 13:01:13 Marc Zyngier wrote:
> > > +/* Routines to acknowledge, disable and enable interrupts */
> > > +static void rps_mask_irq(struct irq_data *d)
> > > +{
> > > +     u32 mask = BIT(d->hwirq);
> > > +
> > > +     iowrite32(mask, rps_data.base + RPS_MASK);
> > 
> > I do question the use of iowrite32 here (and its ioread32 pendent
> > anywhere else), as it actually translates in a writel, which contains a
> > memory barrier. Do you have any case that requires the use of such a
> > barrier? if not, consider switching to relaxed accessors (which are the
> > 
> 
> I really ask everyone to do the opposite: we have seen several drivers
> blindlessly using the relaxed accessors and actually introducing bugs
> that way, so I'd rather see the readl/writel ones used by default.

I actually agree with Marc - we have far too many drivers using the
barriered IO accessors, which are really very expensive on 32-bit
ARM.

For most ARM systems, the rules are quite simple: a write which causes
DMA memory to be accessed by the device must be using the barriered
IO accessor, and a read from a DMA status register must be too.
Everything else need not be.  Barriered IO accessors are only about
access ordering.

That's independent of whether you need a read-back to ensure that the
write has hit the hardware: that's a completely different problem, and
one which is harder for people to understand and get right.  (Eg, for
interrupt registers.)

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

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


#1349443 — Re: [PATCH 02/17] irqchip: Add PLX Technology RPS IRQ Controller

FromArnd Bergmann <arnd@arndb.de>
Date2016-03-03 18:40 +0100
SubjectRe: [PATCH 02/17] irqchip: Add PLX Technology RPS IRQ Controller
Message-ID<r8Fog-25V-41@gated-at.bofh.it>
In reply to#1349159
On Thursday 03 March 2016 13:36:49 Russell King - ARM Linux wrote:
> On Thu, Mar 03, 2016 at 02:08:19PM +0100, Arnd Bergmann wrote:
> > On Thursday 03 March 2016 13:01:13 Marc Zyngier wrote:
> > > > +/* Routines to acknowledge, disable and enable interrupts */
> > > > +static void rps_mask_irq(struct irq_data *d)
> > > > +{
> > > > +     u32 mask = BIT(d->hwirq);
> > > > +
> > > > +     iowrite32(mask, rps_data.base + RPS_MASK);
> > > 
> > > I do question the use of iowrite32 here (and its ioread32 pendent
> > > anywhere else), as it actually translates in a writel, which contains a
> > > memory barrier. Do you have any case that requires the use of such a
> > > barrier? if not, consider switching to relaxed accessors (which are the
> > > 
> > 
> > I really ask everyone to do the opposite: we have seen several drivers
> > blindlessly using the relaxed accessors and actually introducing bugs
> > that way, so I'd rather see the readl/writel ones used by default.
> 
> I actually agree with Marc - we have far too many drivers using the
> barriered IO accessors, which are really very expensive on 32-bit
> ARM.
> 
> For most ARM systems, the rules are quite simple: a write which causes
> DMA memory to be accessed by the device must be using the barriered
> IO accessor, and a read from a DMA status register must be too.
> Everything else need not be.  Barriered IO accessors are only about
> access ordering.

My main worry is really about code getting copied from drivers that
are fine with just relaxed accessors into other drivers by developers
that have never heard about the difference and just want to follow
best practices.

	Arnd

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


#1349144 — Re: [PATCH 02/17] irqchip: Add PLX Technology RPS IRQ Controller

FromMarc Zyngier <marc.zyngier@arm.com>
Date2016-03-03 14:10 +0100
SubjectRe: [PATCH 02/17] irqchip: Add PLX Technology RPS IRQ Controller
Message-ID<r8BaX-7xs-25@gated-at.bofh.it>
In reply to#1349067
Neil,

On 03/03/16 11:39, Neil Armstrong wrote:
> Add PLX Technology RPS IRQ Controller as irqchip driver.
> 
> CC: Ma Haijun <mahaijuns@gmail.com>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/irqchip/Kconfig   |   5 ++
>  drivers/irqchip/Makefile  |   1 +
>  drivers/irqchip/irq-rps.c | 128 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 134 insertions(+)
>  create mode 100644 drivers/irqchip/irq-rps.c
> 
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index fb50911..7892c1a 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -135,6 +135,11 @@ config PIC32_EVIC
>  	select GENERIC_IRQ_CHIP
>  	select IRQ_DOMAIN
>  
> +config PLXTECH_RPS
> +	bool
> +	select GENERIC_IRQ_CHIP
> +	select IRQ_DOMAIN
> +
>  config RENESAS_INTC_IRQPIN
>  	bool
>  	select IRQ_DOMAIN
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index 18caacb..3eec3a0 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -34,6 +34,7 @@ obj-$(CONFIG_I8259)			+= irq-i8259.o
>  obj-$(CONFIG_IMGPDC_IRQ)		+= irq-imgpdc.o
>  obj-$(CONFIG_IRQ_MIPS_CPU)		+= irq-mips-cpu.o
>  obj-$(CONFIG_SIRF_IRQ)			+= irq-sirfsoc.o
> +obj-$(CONFIG_PLXTECH_RPS)		+= irq-rps.o
>  obj-$(CONFIG_RENESAS_INTC_IRQPIN)	+= irq-renesas-intc-irqpin.o
>  obj-$(CONFIG_RENESAS_IRQC)		+= irq-renesas-irqc.o
>  obj-$(CONFIG_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o
> diff --git a/drivers/irqchip/irq-rps.c b/drivers/irqchip/irq-rps.c
> new file mode 100644
> index 0000000..bcd4a31
> --- /dev/null
> +++ b/drivers/irqchip/irq-rps.c
> @@ -0,0 +1,128 @@
> +/*
> + * drivers/irqchip/irq-rps.c
> + *
> + * Copyright (C) 2009 Oxford Semiconductor Ltd
> + * Copyright (C) 2013 Ma Haijun <mahaijuns@gmail.com>
> + * Copyright (C) 2016 Neil Armstrong <narmstrong@baylibre.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/irqdomain.h>
> +#include <linux/irq.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/version.h>
> +#include <linux/irqchip.h>
> +
> +#include <asm/exception.h>
> +
> +struct rps_chip_data {
> +	void __iomem *base;
> +	struct irq_domain *domain;
> +} rps_data;
> +
> +enum {
> +	RPS_IRQ_COUNT = 32,
> +
> +	RPS_STATUS = 0,
> +	RPS_RAW_STATUS = 4,
> +	RPS_UNMASK = 8,
> +	RPS_MASK = 0xc,
> +};

As much as I hate macros (despite making a living out of writing
complicated/braindead ones), shoving random and unrelated values in an
enum doesn't make much sense. Please convert this to a set of #defines.

> +
> +/* Routines to acknowledge, disable and enable interrupts */
> +static void rps_mask_irq(struct irq_data *d)
> +{
> +	u32 mask = BIT(d->hwirq);
> +
> +	iowrite32(mask, rps_data.base + RPS_MASK);

I do question the use of iowrite32 here (and its ioread32 pendent
anywhere else), as it actually translates in a writel, which contains a
memory barrier. Do you have any case that requires the use of such a
barrier? if not, consider switching to relaxed accessors (which are the

> +}
> +
> +static void rps_unmask_irq(struct irq_data *d)
> +{
> +	u32 mask = BIT(d->hwirq);
> +
> +	iowrite32(mask, rps_data.base + RPS_UNMASK);
> +}
> +
> +static void rps_ack_irq(struct irq_data *d)
> +{
> +	/* NOP */
> +}

If that's a nop, you probably don't need it, see below.

> +
> +static void __exception_irq_entry handle_irq(struct pt_regs *regs)
> +{
> +	u32 irqstat;
> +	int hwirq;
> +
> +	irqstat = ioread32(rps_data.base + RPS_STATUS);
> +	hwirq = __ffs(irqstat);
> +
> +	do {
> +		handle_IRQ(irq_find_mapping(rps_data.domain, hwirq), regs);

Please use handle_domain_irq() which will do the right thing (and save
you from RCU shouting at you).

> +
> +		irqstat = ioread32(rps_data.base + RPS_STATUS);
> +		hwirq = __ffs(irqstat);
> +	} while (irqstat);
> +}

Can you get more that a single bit set in one read from the status
register? If so, you'd be better off handling all the pending interrupts
before reading from the MMIO again, since that's a slow operation.

> +
> +int __init rps_of_init(struct device_node *node, struct device_node *parent)
> +{
> +	int ret;
> +	struct irq_chip_generic *gc;
> +
> +	if (WARN_ON(!node))
> +		return -ENODEV;
> +
> +	rps_data.base = of_iomap(node, 0);
> +	WARN(!rps_data.base, "unable to map rps registers\n");
> +
> +	rps_data.domain = irq_domain_add_linear(node, RPS_IRQ_COUNT,
> +						&irq_generic_chip_ops,
> +						NULL);
> +	if (!rps_data.domain) {
> +		pr_err("%s: could add irq domain\n",
> +		       node->full_name);
> +		return -ENOMEM;
> +	}
> +
> +	ret = irq_alloc_domain_generic_chips(rps_data.domain, RPS_IRQ_COUNT, 1,
> +					     "RPS", handle_level_irq,

Given that all your interrupts are level triggered...

> +					     0, 0, IRQ_GC_INIT_NESTED_LOCK);
> +	if (ret) {
> +		pr_err("%s: could not allocate generic chip\n",
> +		       node->full_name);
> +		irq_domain_remove(rps_data.domain);
> +		return -EINVAL;
> +	}
> +
> +	gc = irq_get_domain_generic_chip(rps_data.domain, 0);
> +	gc->chip_types[0].chip.irq_ack = rps_ack_irq;

... I believe you can loose this callback, it is never used by the handler.

> +	gc->chip_types[0].chip.irq_mask = rps_mask_irq;
> +	gc->chip_types[0].chip.irq_unmask = rps_unmask_irq;
> +
> +	/* Disable all IRQs */
> +	iowrite32(~0, rps_data.base + RPS_MASK);
> +
> +	set_handle_irq(handle_irq);
> +
> +	pr_info("Registered %d rps interrupts\n", RPS_IRQ_COUNT);

Given that this is always the same value, I don't think this is a very
useful message...

> +
> +	return 0;
> +}
> +
> +IRQCHIP_DECLARE(nas782x, "plxtech,nas782x-rps", rps_of_init);
> 

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

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


#1349293 — RE: [PATCH 02/17] irqchip: Add PLX Technology RPS IRQ Controller

From"Ma Haijun" <mahaijuns@gmail.com>
Date2016-03-03 16:40 +0100
SubjectRE: [PATCH 02/17] irqchip: Add PLX Technology RPS IRQ Controller
Message-ID<r8Dw7-Di-23@gated-at.bofh.it>
In reply to#1349067
Hi Neil,

Glad to see the mainline efforts of this SoC family.

Previously, I did not really understand what this "RPS" stood for.
After some digging(1)., now I believe it means ARM's Reference Peripheral
Specification
though the spec itself seems not publicly available, the peripheral specs
are accessible.
The interrupt controller is an AMBA Interrupt Controller(2) and the timers
are
AMBA Timer(3). Besides, ARM Dual-Timer Module (SP804)(4) looks like an
extended version
of the AMBA timer

1)
http://infocenter.arm.com/help/topic/com.arm.doc.dai0030a/DAI0030A_sw_int_ap
psnote.pdf
2) http://infocenter.arm.com/help/topic/com.arm.doc.ddi0047d/DDI0047.pdf
3) http://infocenter.arm.com/help/topic/com.arm.doc.ddi0049c/AMBA_Timer.pdf
4) http://infocenter.arm.com/help/topic/com.arm.doc.ddi0271d/DDI0271.pdf

Regards,
Haijun

-----Original Message-----
From: Neil Armstrong [mailto:narmstrong@baylibre.com] 
Subject: [PATCH 02/17] irqchip: Add PLX Technology RPS IRQ Controller

Add PLX Technology RPS IRQ Controller as irqchip driver.

CC: Ma Haijun <mahaijuns@gmail.com>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/irqchip/Kconfig   |   5 ++
 drivers/irqchip/Makefile  |   1 +
 drivers/irqchip/irq-rps.c | 128
++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 134 insertions(+)
 create mode 100644 drivers/irqchip/irq-rps.c

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index
fb50911..7892c1a 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -135,6 +135,11 @@ config PIC32_EVIC
 	select GENERIC_IRQ_CHIP
 	select IRQ_DOMAIN
 
+config PLXTECH_RPS
+	bool
+	select GENERIC_IRQ_CHIP
+	select IRQ_DOMAIN
+
 config RENESAS_INTC_IRQPIN
 	bool
 	select IRQ_DOMAIN
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index
18caacb..3eec3a0 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_I8259)			+= irq-i8259.o
 obj-$(CONFIG_IMGPDC_IRQ)		+= irq-imgpdc.o
 obj-$(CONFIG_IRQ_MIPS_CPU)		+= irq-mips-cpu.o
 obj-$(CONFIG_SIRF_IRQ)			+= irq-sirfsoc.o
+obj-$(CONFIG_PLXTECH_RPS)		+= irq-rps.o
 obj-$(CONFIG_RENESAS_INTC_IRQPIN)	+= irq-renesas-intc-irqpin.o
 obj-$(CONFIG_RENESAS_IRQC)		+= irq-renesas-irqc.o
 obj-$(CONFIG_VERSATILE_FPGA_IRQ)	+= irq-versatile-fpga.o
diff --git a/drivers/irqchip/irq-rps.c b/drivers/irqchip/irq-rps.c new file
mode 100644 index 0000000..bcd4a31
--- /dev/null
+++ b/drivers/irqchip/irq-rps.c
@@ -0,0 +1,128 @@
+/*
+ * drivers/irqchip/irq-rps.c
+ *
+ * Copyright (C) 2009 Oxford Semiconductor Ltd
+ * Copyright (C) 2013 Ma Haijun <mahaijuns@gmail.com>
+ * Copyright (C) 2016 Neil Armstrong <narmstrong@baylibre.com>
+ *
+ * This program is free software; you can redistribute it and/or modify 
+it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/irqdomain.h>
+#include <linux/irq.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/version.h>
+#include <linux/irqchip.h>
+
+#include <asm/exception.h>
+
+struct rps_chip_data {
+	void __iomem *base;
+	struct irq_domain *domain;
+} rps_data;
+
+enum {
+	RPS_IRQ_COUNT = 32,
+
+	RPS_STATUS = 0,
+	RPS_RAW_STATUS = 4,
+	RPS_UNMASK = 8,
+	RPS_MASK = 0xc,
+};
+
+/* Routines to acknowledge, disable and enable interrupts */ static 
+void rps_mask_irq(struct irq_data *d) {
+	u32 mask = BIT(d->hwirq);
+
+	iowrite32(mask, rps_data.base + RPS_MASK); }
+
+static void rps_unmask_irq(struct irq_data *d) {
+	u32 mask = BIT(d->hwirq);
+
+	iowrite32(mask, rps_data.base + RPS_UNMASK); }
+
+static void rps_ack_irq(struct irq_data *d) {
+	/* NOP */
+}
+
+static void __exception_irq_entry handle_irq(struct pt_regs *regs) {
+	u32 irqstat;
+	int hwirq;
+
+	irqstat = ioread32(rps_data.base + RPS_STATUS);
+	hwirq = __ffs(irqstat);
+
+	do {
+		handle_IRQ(irq_find_mapping(rps_data.domain, hwirq), regs);
+
+		irqstat = ioread32(rps_data.base + RPS_STATUS);
+		hwirq = __ffs(irqstat);
+	} while (irqstat);
+}
+
+int __init rps_of_init(struct device_node *node, struct device_node 
+*parent) {
+	int ret;
+	struct irq_chip_generic *gc;
+
+	if (WARN_ON(!node))
+		return -ENODEV;
+
+	rps_data.base = of_iomap(node, 0);
+	WARN(!rps_data.base, "unable to map rps registers\n");
+
+	rps_data.domain = irq_domain_add_linear(node, RPS_IRQ_COUNT,
+						&irq_generic_chip_ops,
+						NULL);
+	if (!rps_data.domain) {
+		pr_err("%s: could add irq domain\n",
+		       node->full_name);
+		return -ENOMEM;
+	}
+
+	ret = irq_alloc_domain_generic_chips(rps_data.domain, RPS_IRQ_COUNT,
1,
+					     "RPS", handle_level_irq,
+					     0, 0, IRQ_GC_INIT_NESTED_LOCK);
+	if (ret) {
+		pr_err("%s: could not allocate generic chip\n",
+		       node->full_name);
+		irq_domain_remove(rps_data.domain);
+		return -EINVAL;
+	}
+
+	gc = irq_get_domain_generic_chip(rps_data.domain, 0);
+	gc->chip_types[0].chip.irq_ack = rps_ack_irq;
+	gc->chip_types[0].chip.irq_mask = rps_mask_irq;
+	gc->chip_types[0].chip.irq_unmask = rps_unmask_irq;
+
+	/* Disable all IRQs */
+	iowrite32(~0, rps_data.base + RPS_MASK);
+
+	set_handle_irq(handle_irq);
+
+	pr_info("Registered %d rps interrupts\n", RPS_IRQ_COUNT);
+
+	return 0;
+}
+
+IRQCHIP_DECLARE(nas782x, "plxtech,nas782x-rps", rps_of_init);
--
1.9.1

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


#1349374 — Re: [PATCH 02/17] irqchip: Add PLX Technology RPS IRQ Controller

FromNeil Armstrong <narmstrong@baylibre.com>
Date2016-03-03 18:00 +0100
SubjectRe: [PATCH 02/17] irqchip: Add PLX Technology RPS IRQ Controller
Message-ID<r8ELx-1ze-45@gated-at.bofh.it>
In reply to#1349293
On 03/03/2016 04:32 PM, Ma Haijun wrote:
> Hi Neil,
> 
> Glad to see the mainline efforts of this SoC family.
> 
> Previously, I did not really understand what this "RPS" stood for.
> After some digging(1)., now I believe it means ARM's Reference Peripheral
> Specification
> though the spec itself seems not publicly available, the peripheral specs
> are accessible.
> The interrupt controller is an AMBA Interrupt Controller(2) and the timers
> are
> AMBA Timer(3). Besides, ARM Dual-Timer Module (SP804)(4) looks like an
> extended version
> of the AMBA timer
> 
> 1)
> http://infocenter.arm.com/help/topic/com.arm.doc.dai0030a/DAI0030A_sw_int_ap
> psnote.pdf
> 2) http://infocenter.arm.com/help/topic/com.arm.doc.ddi0047d/DDI0047.pdf
> 3) http://infocenter.arm.com/help/topic/com.arm.doc.ddi0049c/AMBA_Timer.pdf
> 4) http://infocenter.arm.com/help/topic/com.arm.doc.ddi0271d/DDI0271.pdf
> 
> Regards,
> Haijun

Hi Haijun,

Thanks for the tips !

Indeed the timer looks like the timer-sp804 actually upstream, I'll try this driver before posting a v2.

Concerning the IRQ controller, I did not find any similar drivers.

Marc, Arnd, should I create a specific "arm RPS irq controller" instead ?

Neil

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


#1349418 — Re: [PATCH 02/17] irqchip: Add PLX Technology RPS IRQ Controller

FromMarc Zyngier <marc.zyngier@arm.com>
Date2016-03-03 18:20 +0100
SubjectRe: [PATCH 02/17] irqchip: Add PLX Technology RPS IRQ Controller
Message-ID<r8F4T-1WU-19@gated-at.bofh.it>
In reply to#1349374
On 03/03/16 16:56, Neil Armstrong wrote:
> On 03/03/2016 04:32 PM, Ma Haijun wrote:
>> Hi Neil,
>>
>> Glad to see the mainline efforts of this SoC family.
>>
>> Previously, I did not really understand what this "RPS" stood for.
>> After some digging(1)., now I believe it means ARM's Reference Peripheral
>> Specification
>> though the spec itself seems not publicly available, the peripheral specs
>> are accessible.
>> The interrupt controller is an AMBA Interrupt Controller(2) and the timers
>> are
>> AMBA Timer(3). Besides, ARM Dual-Timer Module (SP804)(4) looks like an
>> extended version
>> of the AMBA timer
>>
>> 1)
>> http://infocenter.arm.com/help/topic/com.arm.doc.dai0030a/DAI0030A_sw_int_ap
>> psnote.pdf
>> 2) http://infocenter.arm.com/help/topic/com.arm.doc.ddi0047d/DDI0047.pdf
>> 3) http://infocenter.arm.com/help/topic/com.arm.doc.ddi0049c/AMBA_Timer.pdf
>> 4) http://infocenter.arm.com/help/topic/com.arm.doc.ddi0271d/DDI0271.pdf
>>
>> Regards,
>> Haijun
> 
> Hi Haijun,
> 
> Thanks for the tips !
> 
> Indeed the timer looks like the timer-sp804 actually upstream, I'll try this driver before posting a v2.
> 
> Concerning the IRQ controller, I did not find any similar drivers.
> 
> Marc, Arnd, should I create a specific "arm RPS irq controller" instead ?

Why not - I wonder which HW this controller got slapped on... It doesn't
even have a cryptic name! ;-)

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

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


#1349069 — [PATCH 09/17] dt-bindings: Add PLX Technology OXNAS Standard Clocks bindings

FromNeil Armstrong <narmstrong@baylibre.com>
Date2016-03-03 13:00 +0100
Subject[PATCH 09/17] dt-bindings: Add PLX Technology OXNAS Standard Clocks bindings
Message-ID<r8A5e-6Bm-57@gated-at.bofh.it>
In reply to#1349048
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 .../devicetree/bindings/clock/plxtech,stdclk.txt   | 24 ++++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/plxtech,stdclk.txt

diff --git a/Documentation/devicetree/bindings/clock/plxtech,stdclk.txt b/Documentation/devicetree/bindings/clock/plxtech,stdclk.txt
new file mode 100644
index 0000000..46465c6
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/plxtech,stdclk.txt
@@ -0,0 +1,24 @@
+PLX Technology OXNAS SoC Family Standard Clocks
+================================================
+
+Please also refer to clock-bindings.txt in this directory for common clock
+bindings usage.
+
+Required properties:
+- compatible: Should be "plxtech,ox810se-stdclk" or "plxtech,nas782x-stdclk"
+- #clock-cells: 1, see below
+
+Parent node should have the following properties :
+- compatible: Should be "plxtech,ox810se-sys-ctrl", "syscon", "simple-mfd"
+
+example:
+
+sys: sys-ctrl@000000 {
+	compatible = "plxtech,ox810se-sys-ctrl", "syscon", "simple-mfd";
+	reg = <0x000000 0x100000>;
+
+	stdclk: stdclk {
+		compatible = "plxtech,ox810se-stdclk", "plxtech,nas782x-stdclk";
+		#reset-cells = <1>;
+	};
+};
-- 
1.9.1

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


#1349097

FromArnd Bergmann <arnd@arndb.de>
Date2016-03-03 13:30 +0100
Message-ID<r8Aye-745-27@gated-at.bofh.it>
In reply to#1349048
On Thursday 03 March 2016 12:39:53 Neil Armstrong wrote:
> This serie adds initial support (IRQ, Timer, GPIO, Reset, Serial, Clocks) for
> the PLX Technology OX810SE used in the well-known Western Digital My Book
> World Edition Network Attached Storage device.
> 
> Extended support for SATA, DMA and Ethernet will come in further patches.
> 
> Upstream support for following devices like the OX820SE is welcome !
> 

Looks very nice overall. I think with the timing, it's a way too
late for 4.6 now, but there should be no problem merging this for 4.7.
I assume it's not urgent on your end, as this seems to be a somewhat
historic platform.

I had no idea there was an ARM9 based predecessor of OX820SE, so I was
surprised to see this one instead. How closely related are they?

	Arnd

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


Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel


csiph-web