2015-04-03 12:42:41

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH/RFC 0/6] staging: board: armadillo800eva: Board staging for sh_mobile_lcdc_fb

Hi all,

This RFC patch series adds board staging support for r8a7740/armadillo.
For now this supports only the frame buffer device for the on-board LCD.
The goal is to complete the move to ARM multiplatform kernels for all
shmobile platforms, and drop the existing board files
(arch/arm/mach-shmobile/board-*).

The board staging area was introduced last year to allow continuous
upstream in-tree development and integration of platform devices. It
helps developers integrate devices as platform devices for device
drivers that only provide platform device bindings. This in turn allows
for incremental development of both hardware feature support and DT
binding work in parallel.

This series consists of 4 parts:
- Patch 1 re-enables compilation of the board staging area, which was
disabled after a compile breakage, but has been fixed in the mean
time,
- Path 2 moves initialization of staging board code to an earlier
moment, as currently it happens after unused PM domains are powered
down,
- Patches 3 and 4 (hopefully) fix the existing kzm9d board staging
code, which was presumably broken by commit 9a1091ef0017c40a
("irqchip: gic: Support hierarchy irq domain."),
- Patches 5 and 6 add support for registering platform devices with
complex dependencies (clocks, pinctrl, gpios, PM domains), and add
armadillo board staging code for enabling a frame buffer on the
on-board LCD.

Questions:
- Are there other devices from board-armadillo.c that work fine in
armadillo-legacy, and that we want to add?
I think this is the list of devices lacking DT support and/or
missing in DTS:
- renesas_usbhs (does this work? the platform device is
instantiated conditionally, but the condition (gpio state) is
never true?)
- sh-mobile-hdmi/sh_mobile_lcdc_fb.1 (this seems to be broken in
-legacy?)
- sdhi1 (we do have sdhi0)
- mmcif
- soc-camera-pdrv/sh_mobile_ceu
- ipmmu
- sh-dma-engine (this will probably never get DT support)

- What about support for other boards (kzm9g, bockw, marzen)?

This was tested on r8a7740/armadillo.
This was not tested on emev2/kzm9d, due to lack of hardware.

Thanks for your comments!

Geert Uytterhoeven (6):
Revert "staging: board: disable as it breaks the build"
[RFC] staging: board: Initialize staging board code earlier
[RFC] staging: board: Add support for translating hwirq to virq numbers
[RFC] staging: board: kzm9d: Translate hwirq numbers to virq numbers
[RFC] staging: board: Add support for devices with complex dependencies
[RFC] staging: board: armadillo800eva: Board staging for sh_mobile_lcdc_fb

drivers/staging/board/Kconfig | 1 -
drivers/staging/board/Makefile | 3 +-
drivers/staging/board/armadillo800eva.c | 124 ++++++++++++++++++++++++++++
drivers/staging/board/board.c | 142 ++++++++++++++++++++++++++++++++
drivers/staging/board/board.h | 38 ++++++++-
drivers/staging/board/kzm9d.c | 10 ++-
6 files changed, 313 insertions(+), 5 deletions(-)
create mode 100644 drivers/staging/board/armadillo800eva.c

--
1.9.1

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


2015-04-03 12:42:38

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH 1/6] Revert "staging: board: disable as it breaks the build"

This reverts commit d13778d537a0ed6115d2a79a942af999cfb8eec6.

Commit 13c11072536f2613 ("staging:board: remove unnecessary function")
fixed the build of drivers/staging/board/board.c.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
drivers/staging/board/Kconfig | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/staging/board/Kconfig b/drivers/staging/board/Kconfig
index 0a89ad16371f7ded..b8ee81840666ad35 100644
--- a/drivers/staging/board/Kconfig
+++ b/drivers/staging/board/Kconfig
@@ -1,7 +1,6 @@
config STAGING_BOARD
bool "Staging Board Support"
depends on OF_ADDRESS
- depends on BROKEN
help
Select to enable per-board staging support code.

--
1.9.1

2015-04-03 12:42:45

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH/RFC 2/6] staging: board: Initialize staging board code earlier

Currently the staging board code is initialized from a late_initcall().
However, unused PM domains are also disabled from a late_initcall(),
which happens before due to link order.

Change the initialization of staging board code from using
late_initcall() to device_initcall() to fix this.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
drivers/staging/board/board.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/board/board.h b/drivers/staging/board/board.h
index 2390ed6c31a42f2f..e9c914985d4acb36 100644
--- a/drivers/staging/board/board.h
+++ b/drivers/staging/board/board.h
@@ -15,6 +15,6 @@ static int __init runtime_board_check(void) \
return 0; \
} \
\
-late_initcall(runtime_board_check)
+device_initcall(runtime_board_check)

#endif /* __BOARD_H__ */
--
1.9.1

2015-04-03 12:43:56

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH/RFC 3/6] staging: board: Add support for translating hwirq to virq numbers

As of commit 9a1091ef0017c40a ("irqchip: gic: Support hierarchy irq
domain."), GIC IRQ numbers are virtual, breaking hardcoded hardware IRQ
numbers in platform device resources.

Add support for translating hardware IRQ numbers to virtual IRQ numbers,
and fixing up platform device resources with hardcoded IRQ numbers.

Add a copyright header, including the original author.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
drivers/staging/board/board.c | 66 +++++++++++++++++++++++++++++++++++++++++++
drivers/staging/board/board.h | 5 ++++
2 files changed, 71 insertions(+)

diff --git a/drivers/staging/board/board.c b/drivers/staging/board/board.c
index d5a6abc845191c93..b84ac2837a20bf06 100644
--- a/drivers/staging/board/board.c
+++ b/drivers/staging/board/board.c
@@ -1,10 +1,27 @@
+/*
+ * Copyright (C) 2014 Magnus Damm
+ * Copyright (C) 2015 Glider bvba
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+
+#define pr_fmt(fmt) "board_staging: " fmt
+
#include <linux/init.h>
+#include <linux/irq.h>
#include <linux/device.h>
#include <linux/kernel.h>
#include <linux/of.h>
#include <linux/of_address.h>
+#include <linux/of_irq.h>
+
#include "board.h"

+static struct device_node *irqc_node __initdata;
+static unsigned int irqc_base __initdata;
+
static bool find_by_address(u64 base_address)
{
struct device_node *dn = of_find_all_nodes(NULL);
@@ -38,3 +55,52 @@ bool __init board_staging_dt_node_available(const struct resource *resource,

return false; /* Nothing found */
}
+
+int __init board_staging_setup_hwirq_xlate(const char *irqc_match,
+ unsigned int base)
+{
+ irqc_node = of_find_compatible_node(NULL, NULL, irqc_match);
+
+ WARN_ON(!irqc_node);
+ if (!irqc_node)
+ return -ENOENT;
+
+ irqc_base = base;
+ return 0;
+}
+
+static unsigned int __init xlate_hwirq(unsigned int hwirq)
+{
+ struct of_phandle_args irq_data;
+ unsigned int irq;
+
+ if (!irqc_node)
+ return hwirq;
+
+ irq_data.np = irqc_node;
+ irq_data.args_count = 3;
+ irq_data.args[0] = 0;
+ irq_data.args[1] = hwirq - irqc_base;
+ irq_data.args[2] = IRQ_TYPE_LEVEL_HIGH;
+
+ irq = irq_create_of_mapping(&irq_data);
+ if (WARN_ON(!irq))
+ irq = hwirq;
+
+ return irq;
+}
+
+void __init board_staging_fixup_irq_resources(struct resource *res,
+ unsigned int nres)
+{
+ unsigned int i;
+
+ for (i = 0; i < nres; i++)
+ if (res[i].flags == IORESOURCE_IRQ) {
+ unsigned int hwirq = res[i].start;
+ unsigned int virq = xlate_hwirq(hwirq);
+
+ pr_debug("hwirq %u -> virq %u\n", hwirq, virq);
+ res[i].start = virq;
+ }
+}
diff --git a/drivers/staging/board/board.h b/drivers/staging/board/board.h
index e9c914985d4acb36..4cedc3c46e287eb7 100644
--- a/drivers/staging/board/board.h
+++ b/drivers/staging/board/board.h
@@ -1,10 +1,15 @@
#ifndef __BOARD_H__
#define __BOARD_H__
+
#include <linux/init.h>
#include <linux/of.h>

+struct resource;
+
bool board_staging_dt_node_available(const struct resource *resource,
unsigned int num_resources);
+int board_staging_setup_hwirq_xlate(const char *irqc_match, unsigned int base);
+void board_staging_fixup_irq_resources(struct resource *res, unsigned int nres);

#define board_staging(str, fn) \
static int __init runtime_board_check(void) \
--
1.9.1

2015-04-03 12:42:13

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH/RFC 4/6] staging: board: kzm9d: Translate hwirq numbers to virq numbers

As of commit 9a1091ef0017c40a ("irqchip: gic: Support hierarchy irq
domain."), GIC IRQ numbers are virtual, breaking hardcoded hardware IRQ
numbers in platform device resources.

Translate the hardware IRQ numbers to virtual IRQ numbers to fix this.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
Untested due to lack of hardware
---
drivers/staging/board/kzm9d.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/board/kzm9d.c b/drivers/staging/board/kzm9d.c
index 533f3026e17aa723..8ea7a1a9bd777dc7 100644
--- a/drivers/staging/board/kzm9d.c
+++ b/drivers/staging/board/kzm9d.c
@@ -4,16 +4,22 @@
#include <linux/platform_device.h>
#include "board.h"

-static const struct resource usbs1_res[] __initconst = {
+static struct resource usbs1_res[] __initdata = {
DEFINE_RES_MEM(0xe2800000, 0x2000),
DEFINE_RES_IRQ(159),
};

static void __init kzm9d_init(void)
{
- if (!board_staging_dt_node_available(usbs1_res, ARRAY_SIZE(usbs1_res)))
+ board_staging_setup_hwirq_xlate("arm,cortex-a9-gic", 32);
+
+ if (!board_staging_dt_node_available(usbs1_res,
+ ARRAY_SIZE(usbs1_res))) {
+ board_staging_fixup_irq_resources(usbs1_res,
+ ARRAY_SIZE(usbs1_res));
platform_device_register_simple("emxx_udc", -1, usbs1_res,
ARRAY_SIZE(usbs1_res));
+ }
}

board_staging("renesas,kzm9d", kzm9d_init);
--
1.9.1

2015-04-03 12:42:16

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH/RFC 5/6] staging: board: Add support for devices with complex dependencies

Add support for easy registering of one ore more platform devices that
may:
- need clocks that are described in DT,
- need pin control configuration,
- rely on a configured GPIO,
- be part of a PM Domain.

All these dependencies are optional.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
drivers/staging/board/board.c | 76 +++++++++++++++++++++++++++++++++++++++++++
drivers/staging/board/board.h | 31 ++++++++++++++++++
2 files changed, 107 insertions(+)

diff --git a/drivers/staging/board/board.c b/drivers/staging/board/board.c
index b84ac2837a20bf06..da2469e2d4262fac 100644
--- a/drivers/staging/board/board.c
+++ b/drivers/staging/board/board.c
@@ -9,6 +9,9 @@

#define pr_fmt(fmt) "board_staging: " fmt

+#include <linux/clk.h>
+#include <linux/clkdev.h>
+#include <linux/gpio.h>
#include <linux/init.h>
#include <linux/irq.h>
#include <linux/device.h>
@@ -16,6 +19,9 @@
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/of_irq.h>
+#include <linux/pinctrl/machine.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>

#include "board.h"

@@ -104,3 +110,73 @@ void __init board_staging_fixup_irq_resources(struct resource *res,
res[i].start = virq;
}
}
+
+int __init board_staging_register_clock(const struct board_staging_clk *bsc)
+{
+ struct clk *clk;
+ int error;
+
+ pr_debug("Registering clock %s for con_id %s dev_id %s\n", bsc->clk,
+ bsc->con_id, bsc->dev_id);
+ clk = clk_get(NULL, bsc->clk);
+ if (IS_ERR(clk)) {
+ error = PTR_ERR(clk);
+ pr_err("Failed to get clock %s (%d)\n", bsc->clk, error);
+ return error;
+ }
+
+ error = clk_register_clkdev(clk, bsc->con_id, bsc->dev_id);
+ if (error)
+ pr_err("Failed to register clock %s (%d)\n", bsc->clk, error);
+ return error;
+
+ clk_put(clk);
+ return 0;
+}
+
+int __init board_staging_register_device(const struct board_staging_dev *dev)
+{
+ struct platform_device *pdev = dev->pdev;
+ unsigned int i;
+ int error;
+
+ pr_debug("Trying to register device %s\n", pdev->name);
+ if (board_staging_dt_node_available(pdev->resource,
+ pdev->num_resources)) {
+ pr_warn("Skipping %s, already in DT\n", pdev->name);
+ return -EEXIST;
+ }
+
+ board_staging_fixup_irq_resources(pdev->resource, pdev->num_resources);
+
+ for (i = 0; i < dev->nclocks; i++)
+ board_staging_register_clock(&dev->clocks[i]);
+
+ if (dev->npinmaps)
+ pinctrl_register_mappings(dev->pinmaps, dev->npinmaps);
+
+ for (i = 0; i < dev->ngpios; i++)
+ gpio_request_one(dev->gpios[i].gpio, dev->gpios[i].flags,
+ pdev->name);
+
+ error = platform_device_register(pdev);
+ if (error) {
+ pr_err("Failed to register device %s (%d)\n", pdev->name,
+ error);
+ return error;
+ }
+
+ if (dev->domain)
+ __pm_genpd_name_add_device(dev->domain, &pdev->dev, NULL);
+
+ return error;
+}
+
+void __init board_staging_register_devices(const struct board_staging_dev *devs,
+ unsigned int ndevs)
+{
+ unsigned int i;
+
+ for (i = 0; i < ndevs; i++)
+ board_staging_register_device(&devs[i]);
+}
diff --git a/drivers/staging/board/board.h b/drivers/staging/board/board.h
index 4cedc3c46e287eb7..7aaa0f7d6fafb9e5 100644
--- a/drivers/staging/board/board.h
+++ b/drivers/staging/board/board.h
@@ -4,12 +4,43 @@
#include <linux/init.h>
#include <linux/of.h>

+struct board_staging_clk {
+ const char *clk;
+ const char *con_id;
+ const char *dev_id;
+};
+
+struct board_staging_gpio {
+ unsigned int gpio;
+ unsigned long flags; /* See GPIOF_* */
+};
+
+struct board_staging_dev {
+ /* Platform Device */
+ struct platform_device *pdev;
+ /* Clocks (optional) */
+ const struct board_staging_clk *clocks;
+ unsigned int nclocks;
+ /* Pin Control Maps (optional) */
+ const struct pinctrl_map *pinmaps;
+ unsigned int npinmaps;
+ /* GPIOs (optional) */
+ const struct board_staging_gpio *gpios;
+ unsigned int ngpios;
+ /* PM Domain (optional) */
+ const char *domain;
+};
+
struct resource;

bool board_staging_dt_node_available(const struct resource *resource,
unsigned int num_resources);
int board_staging_setup_hwirq_xlate(const char *irqc_match, unsigned int base);
void board_staging_fixup_irq_resources(struct resource *res, unsigned int nres);
+int board_staging_register_clock(const struct board_staging_clk *bsc);
+int board_staging_register_device(const struct board_staging_dev *dev);
+void board_staging_register_devices(const struct board_staging_dev *devs,
+ unsigned int ndevs);

#define board_staging(str, fn) \
static int __init runtime_board_check(void) \
--
1.9.1

2015-04-03 12:43:41

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH/RFC 6/6] staging: board: armadillo800eva: Board staging for sh_mobile_lcdc_fb

Add staging board support for the r8a7740-based armadillo800eva board
and add platform devices to allow in-tree continuous development of the
drivers on the armadillo800eva board.

When DT bindings are ready for theses drivers then the platform devices
in the armadillo800eva staging board code can easily be removed. Until
then we use platform devices to continuously improve the driver and
integrate code.

Added platform devices:
- sh_mobile_lcdc_fb for the on-board LCD.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
What other devices do we want to add?
---
drivers/staging/board/Makefile | 3 +-
drivers/staging/board/armadillo800eva.c | 124 ++++++++++++++++++++++++++++++++
2 files changed, 126 insertions(+), 1 deletion(-)
create mode 100644 drivers/staging/board/armadillo800eva.c

diff --git a/drivers/staging/board/Makefile b/drivers/staging/board/Makefile
index 65d39ecfad63bbf4..6842745feb9404f8 100644
--- a/drivers/staging/board/Makefile
+++ b/drivers/staging/board/Makefile
@@ -1,2 +1,3 @@
obj-y := board.o
-obj-$(CONFIG_ARCH_EMEV2) += kzm9d.o
+obj-$(CONFIG_ARCH_EMEV2) += kzm9d.o
+obj-$(CONFIG_ARCH_R8A7740) += armadillo800eva.o
diff --git a/drivers/staging/board/armadillo800eva.c b/drivers/staging/board/armadillo800eva.c
new file mode 100644
index 0000000000000000..f9cb645877f52c52
--- /dev/null
+++ b/drivers/staging/board/armadillo800eva.c
@@ -0,0 +1,124 @@
+/*
+ * Staging board support for Armadillo 800 eva.
+ * Enable not-yet-DT-capable devices here.
+ *
+ * Based on board-armadillo800eva.c
+ *
+ * Copyright (C) 2012 Renesas Solutions Corp.
+ * Copyright (C) 2012 Kuninori Morimoto <[email protected]>
+ *
+ * This program 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; version 2 of the License.
+ *
+ * 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/dma-mapping.h>
+#include <linux/fb.h>
+#include <linux/gpio.h>
+#include <linux/kernel.h>
+#include <linux/pinctrl/machine.h>
+#include <linux/platform_device.h>
+#include <linux/videodev2.h>
+
+#include <video/sh_mobile_lcdc.h>
+
+#include "board.h"
+
+
+static struct fb_videomode lcdc0_mode = {
+ .name = "AMPIER/AM-800480",
+ .xres = 800,
+ .yres = 480,
+ .left_margin = 88,
+ .right_margin = 40,
+ .hsync_len = 128,
+ .upper_margin = 20,
+ .lower_margin = 5,
+ .vsync_len = 5,
+ .sync = 0,
+};
+
+static struct sh_mobile_lcdc_info lcdc0_info = {
+ .clock_source = LCDC_CLK_BUS,
+ .ch[0] = {
+ .chan = LCDC_CHAN_MAINLCD,
+ .fourcc = V4L2_PIX_FMT_RGB565,
+ .interface_type = RGB24,
+ .clock_divider = 5,
+ .flags = 0,
+ .lcd_modes = &lcdc0_mode,
+ .num_modes = 1,
+ .panel_cfg = {
+ .width = 111,
+ .height = 68,
+ },
+ },
+};
+
+static struct resource lcdc0_resources[] = {
+ [0] = {
+ .name = "LCD0",
+ .start = 0xfe940000,
+ .end = 0xfe943fff,
+ .flags = IORESOURCE_MEM,
+ },
+ [1] = {
+ .start = 177 + 32,
+ .flags = IORESOURCE_IRQ,
+ },
+};
+
+static struct platform_device lcdc0_device = {
+ .name = "sh_mobile_lcdc_fb",
+ .num_resources = ARRAY_SIZE(lcdc0_resources),
+ .resource = lcdc0_resources,
+ .id = 0,
+ .dev = {
+ .platform_data = &lcdc0_info,
+ .coherent_dma_mask = DMA_BIT_MASK(32),
+ },
+};
+
+static const struct board_staging_clk lcdc0_clocks[] __initconst = {
+ { "lcdc0", NULL, "sh_mobile_lcdc_fb.0" },
+};
+
+static const struct pinctrl_map lcdc0_pinmaps[] __initconst = {
+ PIN_MAP_MUX_GROUP_DEFAULT("sh_mobile_lcdc_fb.0", "e6050000.pfc",
+ "lcd0_data24_0", "lcd0"),
+ PIN_MAP_MUX_GROUP_DEFAULT("sh_mobile_lcdc_fb.0", "e6050000.pfc",
+ "lcd0_lclk_1", "lcd0"),
+ PIN_MAP_MUX_GROUP_DEFAULT("sh_mobile_lcdc_fb.0", "e6050000.pfc",
+ "lcd0_sync", "lcd0"),
+};
+
+static const struct board_staging_gpio lcdc0_gpios[] __initconst = {
+ { 176, GPIOF_OUT_INIT_HIGH }, /* DBGMD/LCDC0/FSIA MUX */
+};
+
+static const struct board_staging_dev armadillo800eva_devices[] __initconst = {
+ {
+ .pdev = &lcdc0_device,
+ .clocks = lcdc0_clocks,
+ .nclocks = ARRAY_SIZE(lcdc0_clocks),
+ .pinmaps = lcdc0_pinmaps,
+ .npinmaps = ARRAY_SIZE(lcdc0_pinmaps),
+ .gpios = lcdc0_gpios,
+ .ngpios = ARRAY_SIZE(lcdc0_gpios),
+ .domain = "a4lc",
+ },
+};
+
+static void __init armadillo800eva_init(void)
+{
+ board_staging_setup_hwirq_xlate("arm,cortex-a9-gic", 32);
+ board_staging_register_devices(armadillo800eva_devices,
+ ARRAY_SIZE(armadillo800eva_devices));
+}
+
+board_staging("renesas,armadillo800eva", armadillo800eva_init);
--
1.9.1

2015-04-03 13:00:33

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH/RFC 5/6] staging: board: Add support for devices with complex dependencies

On Fri, Apr 03, 2015 at 02:42:02PM +0200, Geert Uytterhoeven wrote:
> +int __init board_staging_register_clock(const struct board_staging_clk *bsc)
> +{
> + struct clk *clk;
> + int error;
> +
> + pr_debug("Registering clock %s for con_id %s dev_id %s\n", bsc->clk,
> + bsc->con_id, bsc->dev_id);
> + clk = clk_get(NULL, bsc->clk);
> + if (IS_ERR(clk)) {
> + error = PTR_ERR(clk);
> + pr_err("Failed to get clock %s (%d)\n", bsc->clk, error);
> + return error;
> + }
> +
> + error = clk_register_clkdev(clk, bsc->con_id, bsc->dev_id);
> + if (error)
> + pr_err("Failed to register clock %s (%d)\n", bsc->clk, error);
> + return error;

Missing curly braces. Also it's weird that don't we need a clk_put()
on the error patch as well as the success path?

> +
> + clk_put(clk);
> + return 0;
> +}


regards,
dan carpenter

2015-04-03 13:27:44

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH/RFC 5/6] staging: board: Add support for devices with complex dependencies

On Fri, Apr 3, 2015 at 2:57 PM, Dan Carpenter <[email protected]> wrote:
>> + error = clk_register_clkdev(clk, bsc->con_id, bsc->dev_id);
>> + if (error)
>> + pr_err("Failed to register clock %s (%d)\n", bsc->clk, error);
>> + return error;
>
> Missing curly braces. Also it's weird that don't we need a clk_put()
> on the error patch as well as the success path?

Thanks!

So it worked only by accident: with the new per-user struct clk instances
clk_put() must not be called if clk_register_clkdev() succeeded.

Will call clk_put() only if it failed.

>> +
>> + clk_put(clk);
>> + return 0;

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2015-04-03 16:24:28

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH/RFC 0/6] staging: board: armadillo800eva: Board staging for sh_mobile_lcdc_fb

Hello Geert,

Thank you for the patches.

On Friday 03 April 2015 14:41:57 Geert Uytterhoeven wrote:
> Hi all,
>
> This RFC patch series adds board staging support for r8a7740/armadillo.
> For now this supports only the frame buffer device for the on-board LCD.

I've started adding DT support to the shmob-lcdc driver, but I have no board
to test it on. My Armadillo is gone, and KZM9G is unusable with DT + LCDC at
the moment as the LCDC interrupt is connected to the legacy interrupt
controller only, not the GIC. Would you be able to test a patch set ? And if
you want to implement support for the KZM9G legacy interrupt controller in DT,
please go for it :-)

> The goal is to complete the move to ARM multiplatform kernels for all
> shmobile platforms, and drop the existing board files
> (arch/arm/mach-shmobile/board-*).
>
> The board staging area was introduced last year to allow continuous
> upstream in-tree development and integration of platform devices. It
> helps developers integrate devices as platform devices for device
> drivers that only provide platform device bindings. This in turn allows
> for incremental development of both hardware feature support and DT
> binding work in parallel.
>
> This series consists of 4 parts:
> - Patch 1 re-enables compilation of the board staging area, which was
> disabled after a compile breakage, but has been fixed in the mean
> time,
> - Path 2 moves initialization of staging board code to an earlier
> moment, as currently it happens after unused PM domains are powered
> down,
> - Patches 3 and 4 (hopefully) fix the existing kzm9d board staging
> code, which was presumably broken by commit 9a1091ef0017c40a
> ("irqchip: gic: Support hierarchy irq domain."),
> - Patches 5 and 6 add support for registering platform devices with
> complex dependencies (clocks, pinctrl, gpios, PM domains), and add
> armadillo board staging code for enabling a frame buffer on the
> on-board LCD.
>
> Questions:
> - Are there other devices from board-armadillo.c that work fine in
> armadillo-legacy, and that we want to add?
> I think this is the list of devices lacking DT support and/or
> missing in DTS:
> - renesas_usbhs (does this work? the platform device is
> instantiated conditionally, but the condition (gpio state) is
> never true?)
> - sh-mobile-hdmi/sh_mobile_lcdc_fb.1 (this seems to be broken in
> -legacy?)

HDMI is broken in legacy, we can thus drop it.

> - sdhi1 (we do have sdhi0)
> - mmcif
> - soc-camera-pdrv/sh_mobile_ceu
> - ipmmu
> - sh-dma-engine (this will probably never get DT support)
>
> - What about support for other boards (kzm9g, bockw, marzen)?

At this point it becomes a budget question, we can certainly port them to DT,
but it might not make sense to spend time on that. I'll let Magnus chime in.

> This was tested on r8a7740/armadillo.
> This was not tested on emev2/kzm9d, due to lack of hardware.
>
> Thanks for your comments!
>
> Geert Uytterhoeven (6):
> Revert "staging: board: disable as it breaks the build"
> [RFC] staging: board: Initialize staging board code earlier
> [RFC] staging: board: Add support for translating hwirq to virq numbers
> [RFC] staging: board: kzm9d: Translate hwirq numbers to virq numbers
> [RFC] staging: board: Add support for devices with complex dependencies
> [RFC] staging: board: armadillo800eva: Board staging for sh_mobile_lcdc_fb
>
> drivers/staging/board/Kconfig | 1 -
> drivers/staging/board/Makefile | 3 +-
> drivers/staging/board/armadillo800eva.c | 124 ++++++++++++++++++++++++++++
> drivers/staging/board/board.c | 142 +++++++++++++++++++++++++++++
> drivers/staging/board/board.h | 38 ++++++++-
> drivers/staging/board/kzm9d.c | 10 ++-
> 6 files changed, 313 insertions(+), 5 deletions(-)
> create mode 100644 drivers/staging/board/armadillo800eva.c

--
Regards,

Laurent Pinchart

2015-04-03 17:04:38

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH/RFC 5/6] staging: board: Add support for devices with complex dependencies

On Fri, Apr 03, 2015 at 03:57:27PM +0300, Dan Carpenter wrote:
> On Fri, Apr 03, 2015 at 02:42:02PM +0200, Geert Uytterhoeven wrote:
> > +int __init board_staging_register_clock(const struct board_staging_clk *bsc)
> > +{
> > + struct clk *clk;
> > + int error;
> > +
> > + pr_debug("Registering clock %s for con_id %s dev_id %s\n", bsc->clk,
> > + bsc->con_id, bsc->dev_id);
> > + clk = clk_get(NULL, bsc->clk);
> > + if (IS_ERR(clk)) {
> > + error = PTR_ERR(clk);
> > + pr_err("Failed to get clock %s (%d)\n", bsc->clk, error);
> > + return error;
> > + }
> > +
> > + error = clk_register_clkdev(clk, bsc->con_id, bsc->dev_id);
> > + if (error)
> > + pr_err("Failed to register clock %s (%d)\n", bsc->clk, error);
> > + return error;
>
> Missing curly braces. Also it's weird that don't we need a clk_put()
> on the error patch as well as the success path?

What's also concerning is that this is an abuse of this.

clk_register_clkdev() is supposed to be used with clocks created with
the CCF functions, it's not for creating aliases.

We have clk_add_alias() which does *everything* that this function does,
only in a less buggy way.

--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

2015-04-03 17:08:18

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH/RFC 5/6] staging: board: Add support for devices with complex dependencies

On Fri, Apr 03, 2015 at 03:27:40PM +0200, Geert Uytterhoeven wrote:
> On Fri, Apr 3, 2015 at 2:57 PM, Dan Carpenter <[email protected]> wrote:
> >> + error = clk_register_clkdev(clk, bsc->con_id, bsc->dev_id);
> >> + if (error)
> >> + pr_err("Failed to register clock %s (%d)\n", bsc->clk, error);
> >> + return error;
> >
> > Missing curly braces. Also it's weird that don't we need a clk_put()
> > on the error patch as well as the success path?
>
> Thanks!
>
> So it worked only by accident: with the new per-user struct clk instances
> clk_put() must not be called if clk_register_clkdev() succeeded.

Yes, that's because the per-user struct clk messed quite a lot of things
up - the patches were /not/ well tested before they went in.

That's no excuse to work around the breakage they caused though.

That said, I never did post the work I did earlier this month to fix the
problems in clkdev which those patches caused... so, I guess it's time
to post them and rush them in for the 4.1 merge window... (frankly, the
per-user struct clk patches should've been reverted.)

--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

2015-04-03 19:35:28

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH/RFC 0/6] staging: board: armadillo800eva: Board staging for sh_mobile_lcdc_fb

Hi Laurent,

On Fri, Apr 3, 2015 at 6:24 PM, Laurent Pinchart
<[email protected]> wrote:
> On Friday 03 April 2015 14:41:57 Geert Uytterhoeven wrote:
>> This RFC patch series adds board staging support for r8a7740/armadillo.
>> For now this supports only the frame buffer device for the on-board LCD.
>
> I've started adding DT support to the shmob-lcdc driver, but I have no board
> to test it on. My Armadillo is gone, and KZM9G is unusable with DT + LCDC at
> the moment as the LCDC interrupt is connected to the legacy interrupt
> controller only, not the GIC. Would you be able to test a patch set ? And if

Sure, I can test your patches on Armadillo.

> you want to implement support for the KZM9G legacy interrupt controller in DT,
> please go for it :-)

Do we need the LCDC interrupt? ;-)

Anyway, the display of my kzm9g is broken, so I never had the joy of
seeing its marvellous graphics.

>> - sh-mobile-hdmi/sh_mobile_lcdc_fb.1 (this seems to be broken in
>> -legacy?)
>
> HDMI is broken in legacy, we can thus drop it.

In addition, clk-r8a7740 lacks support for the non-standard register layout of
HDMICKCR.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2015-04-04 12:46:39

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH/RFC 5/6] staging: board: Add support for devices with complex dependencies

Hi Geert,

Thank you for the patch.

On Friday 03 April 2015 14:42:02 Geert Uytterhoeven wrote:
> Add support for easy registering of one ore more platform devices that
> may:
> - need clocks that are described in DT,
> - need pin control configuration,
> - rely on a configured GPIO,
> - be part of a PM Domain.
>
> All these dependencies are optional.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> drivers/staging/board/board.c | 76 ++++++++++++++++++++++++++++++++++++++++
> drivers/staging/board/board.h | 31 ++++++++++++++++++
> 2 files changed, 107 insertions(+)
>
> diff --git a/drivers/staging/board/board.c b/drivers/staging/board/board.c
> index b84ac2837a20bf06..da2469e2d4262fac 100644
> --- a/drivers/staging/board/board.c
> +++ b/drivers/staging/board/board.c
> @@ -9,6 +9,9 @@
>
> #define pr_fmt(fmt) "board_staging: " fmt
>
> +#include <linux/clk.h>
> +#include <linux/clkdev.h>
> +#include <linux/gpio.h>
> #include <linux/init.h>
> #include <linux/irq.h>
> #include <linux/device.h>
> @@ -16,6 +19,9 @@
> #include <linux/of.h>
> #include <linux/of_address.h>
> #include <linux/of_irq.h>
> +#include <linux/pinctrl/machine.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
>
> #include "board.h"
>
> @@ -104,3 +110,73 @@ void __init board_staging_fixup_irq_resources(struct
> resource *res, res[i].start = virq;
> }
> }
> +
> +int __init board_staging_register_clock(const struct board_staging_clk
> *bsc) +{
> + struct clk *clk;
> + int error;
> +
> + pr_debug("Registering clock %s for con_id %s dev_id %s\n", bsc->clk,
> + bsc->con_id, bsc->dev_id);
> + clk = clk_get(NULL, bsc->clk);
> + if (IS_ERR(clk)) {
> + error = PTR_ERR(clk);
> + pr_err("Failed to get clock %s (%d)\n", bsc->clk, error);
> + return error;
> + }
> +
> + error = clk_register_clkdev(clk, bsc->con_id, bsc->dev_id);
> + if (error)
> + pr_err("Failed to register clock %s (%d)\n", bsc->clk, error);
> + return error;
> +
> + clk_put(clk);
> + return 0;
> +}
> +
> +int __init board_staging_register_device(const struct board_staging_dev
> *dev) +{
> + struct platform_device *pdev = dev->pdev;
> + unsigned int i;
> + int error;
> +
> + pr_debug("Trying to register device %s\n", pdev->name);
> + if (board_staging_dt_node_available(pdev->resource,
> + pdev->num_resources)) {
> + pr_warn("Skipping %s, already in DT\n", pdev->name);
> + return -EEXIST;
> + }
> +
> + board_staging_fixup_irq_resources(pdev->resource, pdev->num_resources);
> +
> + for (i = 0; i < dev->nclocks; i++)
> + board_staging_register_clock(&dev->clocks[i]);
> +
> + if (dev->npinmaps)
> + pinctrl_register_mappings(dev->pinmaps, dev->npinmaps);
> +
> + for (i = 0; i < dev->ngpios; i++)
> + gpio_request_one(dev->gpios[i].gpio, dev->gpios[i].flags,
> + pdev->name);

Aren't GPIO numbers dynamic too in DT-based systems ? Beside, shouldn't it be
the responsibility of the drievr to request the GPIOs it needs ?

> + error = platform_device_register(pdev);
> + if (error) {
> + pr_err("Failed to register device %s (%d)\n", pdev->name,
> + error);
> + return error;
> + }
> +
> + if (dev->domain)
> + __pm_genpd_name_add_device(dev->domain, &pdev->dev, NULL);
> +
> + return error;
> +}
> +
> +void __init board_staging_register_devices(const struct board_staging_dev
> *devs, + unsigned int ndevs)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < ndevs; i++)
> + board_staging_register_device(&devs[i]);
> +}
> diff --git a/drivers/staging/board/board.h b/drivers/staging/board/board.h
> index 4cedc3c46e287eb7..7aaa0f7d6fafb9e5 100644
> --- a/drivers/staging/board/board.h
> +++ b/drivers/staging/board/board.h
> @@ -4,12 +4,43 @@
> #include <linux/init.h>
> #include <linux/of.h>
>
> +struct board_staging_clk {
> + const char *clk;
> + const char *con_id;
> + const char *dev_id;
> +};
> +
> +struct board_staging_gpio {
> + unsigned int gpio;
> + unsigned long flags; /* See GPIOF_* */
> +};
> +
> +struct board_staging_dev {
> + /* Platform Device */
> + struct platform_device *pdev;
> + /* Clocks (optional) */
> + const struct board_staging_clk *clocks;
> + unsigned int nclocks;
> + /* Pin Control Maps (optional) */
> + const struct pinctrl_map *pinmaps;
> + unsigned int npinmaps;
> + /* GPIOs (optional) */
> + const struct board_staging_gpio *gpios;
> + unsigned int ngpios;
> + /* PM Domain (optional) */
> + const char *domain;
> +};
> +
> struct resource;
>
> bool board_staging_dt_node_available(const struct resource *resource,
> unsigned int num_resources);
> int board_staging_setup_hwirq_xlate(const char *irqc_match, unsigned int
> base); void board_staging_fixup_irq_resources(struct resource *res,
> unsigned int nres); +int board_staging_register_clock(const struct
> board_staging_clk *bsc); +int board_staging_register_device(const struct
> board_staging_dev *dev); +void board_staging_register_devices(const struct
> board_staging_dev *devs, + unsigned int ndevs);
>
> #define board_staging(str, fn) \
> static int __init runtime_board_check(void) \

--
Regards,

Laurent Pinchart

2015-04-05 08:55:11

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH/RFC 5/6] staging: board: Add support for devices with complex dependencies

Hi Russell,

On Fri, Apr 3, 2015 at 7:04 PM, Russell King - ARM Linux
<[email protected]> wrote:
> On Fri, Apr 03, 2015 at 03:57:27PM +0300, Dan Carpenter wrote:
>> On Fri, Apr 03, 2015 at 02:42:02PM +0200, Geert Uytterhoeven wrote:
>> > +int __init board_staging_register_clock(const struct board_staging_clk *bsc)
>> > +{
>> > + struct clk *clk;
>> > + int error;
>> > +
>> > + pr_debug("Registering clock %s for con_id %s dev_id %s\n", bsc->clk,
>> > + bsc->con_id, bsc->dev_id);
>> > + clk = clk_get(NULL, bsc->clk);
>> > + if (IS_ERR(clk)) {
>> > + error = PTR_ERR(clk);
>> > + pr_err("Failed to get clock %s (%d)\n", bsc->clk, error);
>> > + return error;
>> > + }
>> > +
>> > + error = clk_register_clkdev(clk, bsc->con_id, bsc->dev_id);
>> > + if (error)
>> > + pr_err("Failed to register clock %s (%d)\n", bsc->clk, error);
>> > + return error;
>>
>> Missing curly braces. Also it's weird that don't we need a clk_put()
>> on the error patch as well as the success path?
>
> What's also concerning is that this is an abuse of this.
>
> clk_register_clkdev() is supposed to be used with clocks created with
> the CCF functions, it's not for creating aliases.
>
> We have clk_add_alias() which does *everything* that this function does,
> only in a less buggy way.

Thanks, I didn't know about clk_add_alias(). I had based the above on long gone
code under arch/arm/mach-shmobile to use platform devices with CCF.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2015-04-05 09:01:01

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH/RFC 5/6] staging: board: Add support for devices with complex dependencies

Hi Laurent,

On Sat, Apr 4, 2015 at 2:46 PM, Laurent Pinchart
<[email protected]> wrote:
>> + for (i = 0; i < dev->ngpios; i++)
>> + gpio_request_one(dev->gpios[i].gpio, dev->gpios[i].flags,
>> + pdev->name);
>
> Aren't GPIO numbers dynamic too in DT-based systems ? Beside, shouldn't it be

Apparently not, as the old legacy number still works, and it doesn't work
without.

> the responsibility of the drievr to request the GPIOs it needs ?

As far as I understand it, on Armadillo this is used more for platform
configuration than for device configuration, as it affects multiple devices
(the comment says DBGMD/LCDC0/FSIA MUX).

I guess I could use a "gpio-hog" subnode in DT instead, but then we're already
implementing the conversion to DT ;-)

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2015-04-05 20:06:02

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH/RFC 5/6] staging: board: Add support for devices with complex dependencies

Hi Geert,

On Sunday 05 April 2015 11:00:56 Geert Uytterhoeven wrote:
> On Sat, Apr 4, 2015 at 2:46 PM, Laurent Pinchart wrote:
> >> + for (i = 0; i < dev->ngpios; i++)
> >> + gpio_request_one(dev->gpios[i].gpio, dev->gpios[i].flags,
> >> + pdev->name);
> >
> > Aren't GPIO numbers dynamic too in DT-based systems ? Beside, shouldn't it
> > be
>
> Apparently not, as the old legacy number still works, and it doesn't work
> without.

I think we're just lucky there that the SoC main GPIO controller gets
registered first and starts counting GPIOs with a zero offset.

> > the responsibility of the drievr to request the GPIOs it needs ?
>
> As far as I understand it, on Armadillo this is used more for platform
> configuration than for device configuration, as it affects multiple devices
> (the comment says DBGMD/LCDC0/FSIA MUX).
>
> I guess I could use a "gpio-hog" subnode in DT instead, but then we're
> already implementing the conversion to DT ;-)

But that's the goal :-) I'd rather move GPIO and pinctrl to DT directly as we
already have the infrastructure to do so.

--
Regards,

Laurent Pinchart

2015-04-06 00:40:40

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH 1/6] Revert "staging: board: disable as it breaks the build"

On Fri, Apr 03, 2015 at 02:41:58PM +0200, Geert Uytterhoeven wrote:
> This reverts commit d13778d537a0ed6115d2a79a942af999cfb8eec6.
>
> Commit 13c11072536f2613 ("staging:board: remove unnecessary function")
> fixed the build of drivers/staging/board/board.c.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>

Signed-off-by: Simon Horman <[email protected]>

> ---
> drivers/staging/board/Kconfig | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/staging/board/Kconfig b/drivers/staging/board/Kconfig
> index 0a89ad16371f7ded..b8ee81840666ad35 100644
> --- a/drivers/staging/board/Kconfig
> +++ b/drivers/staging/board/Kconfig
> @@ -1,7 +1,6 @@
> config STAGING_BOARD
> bool "Staging Board Support"
> depends on OF_ADDRESS
> - depends on BROKEN
> help
> Select to enable per-board staging support code.
>
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2015-04-06 10:49:38

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH/RFC 0/6] staging: board: armadillo800eva: Board staging for sh_mobile_lcdc_fb

Hi Geert,

On 2015-04-03 13:41, Geert Uytterhoeven wrote:
> Hi all,
>
> This RFC patch series adds board staging support for
> r8a7740/armadillo.
> For now this supports only the frame buffer device for the on-board
> LCD.
> The goal is to complete the move to ARM multiplatform kernels for all
> shmobile platforms, and drop the existing board files
> (arch/arm/mach-shmobile/board-*).
>
> The board staging area was introduced last year to allow continuous
> upstream in-tree development and integration of platform devices. It
> helps developers integrate devices as platform devices for device
> drivers that only provide platform device bindings. This in turn
> allows
> for incremental development of both hardware feature support and DT
> binding work in parallel.
>
> This series consists of 4 parts:
> - Patch 1 re-enables compilation of the board staging area, which
> was
> disabled after a compile breakage, but has been fixed in the mean
> time,
> - Path 2 moves initialization of staging board code to an earlier
> moment, as currently it happens after unused PM domains are
> powered
> down,
> - Patches 3 and 4 (hopefully) fix the existing kzm9d board staging
> code, which was presumably broken by commit 9a1091ef0017c40a
> ("irqchip: gic: Support hierarchy irq domain."),

Please allow me a semantic correction here: this commit never broke
anything. It merely revealed how platform code (OMAP, iMX6, shmobile)
abused the irq_domain subsystem, hoping to get away with only a partial
conversion to DT. The platform code itself was broken from the moment it
used DT to discover its interrupt controller.

The truth is, it is not possible to sanely convert bits of a platform
to DT. The changes creep into all the subsystems, and doing it in stages
requires tons of ugly hacks (and I've written my share of them).

That being said, I'm off reviewing these two matches.

Thanks,

M.
--
Fast, cheap, reliable. Pick two.

2015-04-06 10:45:57

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH/RFC 3/6] staging: board: Add support for translating hwirq to virq numbers

On 2015-04-03 13:42, Geert Uytterhoeven wrote:
> As of commit 9a1091ef0017c40a ("irqchip: gic: Support hierarchy irq
> domain."), GIC IRQ numbers are virtual, breaking hardcoded hardware
> IRQ
> numbers in platform device resources.
>
> Add support for translating hardware IRQ numbers to virtual IRQ
> numbers,
> and fixing up platform device resources with hardcoded IRQ numbers.
>
> Add a copyright header, including the original author.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> drivers/staging/board/board.c | 66
> +++++++++++++++++++++++++++++++++++++++++++
> drivers/staging/board/board.h | 5 ++++
> 2 files changed, 71 insertions(+)
>
> diff --git a/drivers/staging/board/board.c
> b/drivers/staging/board/board.c
> index d5a6abc845191c93..b84ac2837a20bf06 100644
> --- a/drivers/staging/board/board.c
> +++ b/drivers/staging/board/board.c
> @@ -1,10 +1,27 @@
> +/*
> + * Copyright (C) 2014 Magnus Damm
> + * Copyright (C) 2015 Glider bvba
> + *
> + * This file is subject to the terms and conditions of the GNU
> General Public
> + * License. See the file "COPYING" in the main directory of this
> archive
> + * for more details.
> + */
> +
> +#define pr_fmt(fmt) "board_staging: " fmt
> +
> #include <linux/init.h>
> +#include <linux/irq.h>
> #include <linux/device.h>
> #include <linux/kernel.h>
> #include <linux/of.h>
> #include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +
> #include "board.h"
>
> +static struct device_node *irqc_node __initdata;
> +static unsigned int irqc_base __initdata;
> +
> static bool find_by_address(u64 base_address)
> {
> struct device_node *dn = of_find_all_nodes(NULL);
> @@ -38,3 +55,52 @@ bool __init board_staging_dt_node_available(const
> struct resource *resource,
>
> return false; /* Nothing found */
> }
> +
> +int __init board_staging_setup_hwirq_xlate(const char *irqc_match,
> + unsigned int base)
> +{
> + irqc_node = of_find_compatible_node(NULL, NULL, irqc_match);
> +
> + WARN_ON(!irqc_node);
> + if (!irqc_node)
> + return -ENOENT;
> +
> + irqc_base = base;
> + return 0;
> +}

And what happens when you have multiple (cascaded) interrupt
controllers, each wanting to register with this translation service? You
should at least check that nobody has been here before.

> +static unsigned int __init xlate_hwirq(unsigned int hwirq)
> +{
> + struct of_phandle_args irq_data;
> + unsigned int irq;
> +
> + if (!irqc_node)
> + return hwirq;
> +
> + irq_data.np = irqc_node;
> + irq_data.args_count = 3;
> + irq_data.args[0] = 0;
> + irq_data.args[1] = hwirq - irqc_base;
> + irq_data.args[2] = IRQ_TYPE_LEVEL_HIGH;

And what happens for edge interrupts? Or LEVEL_LOW? This is very much
GIC specific (3 args...). How does it work with non-GIC interrupt
controllers?

> +
> + irq = irq_create_of_mapping(&irq_data);
> + if (WARN_ON(!irq))
> + irq = hwirq;
> +
> + return irq;
> +}
> +
> +void __init board_staging_fixup_irq_resources(struct resource *res,
> + unsigned int nres)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < nres; i++)
> + if (res[i].flags == IORESOURCE_IRQ) {
> + unsigned int hwirq = res[i].start;
> + unsigned int virq = xlate_hwirq(hwirq);
> +
> + pr_debug("hwirq %u -> virq %u\n", hwirq, virq);
> + res[i].start = virq;
> + }
> +}
> diff --git a/drivers/staging/board/board.h
> b/drivers/staging/board/board.h
> index e9c914985d4acb36..4cedc3c46e287eb7 100644
> --- a/drivers/staging/board/board.h
> +++ b/drivers/staging/board/board.h
> @@ -1,10 +1,15 @@
> #ifndef __BOARD_H__
> #define __BOARD_H__
> +
> #include <linux/init.h>
> #include <linux/of.h>
>
> +struct resource;
> +
> bool board_staging_dt_node_available(const struct resource
> *resource,
> unsigned int num_resources);
> +int board_staging_setup_hwirq_xlate(const char *irqc_match, unsigned
> int base);
> +void board_staging_fixup_irq_resources(struct resource *res,
> unsigned int nres);
>
> #define board_staging(str, fn) \
> static int __init runtime_board_check(void) \

It won't surprise you that I don't really like this approach. It is
controller-specific, restrictive, and allows platform code to continue
doing something that is essentially wrong. Should this code ever move
out of staging, it should depend on CONFIG_BROKEN, because this is
essentially what it is - support code for broken systems. I'd also
welcome moving parts of the OMAP4/5 code to such CONFIG_BROKEN
dependency.

Thanks,

M.
--
Fast, cheap, reliable. Pick two.

2015-04-07 13:17:11

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 1/6] Revert "staging: board: disable as it breaks the build"

On Mon, Apr 6, 2015 at 2:40 AM, Simon Horman <[email protected]> wrote:
> On Fri, Apr 03, 2015 at 02:41:58PM +0200, Geert Uytterhoeven wrote:
>> This reverts commit d13778d537a0ed6115d2a79a942af999cfb8eec6.
>>
>> Commit 13c11072536f2613 ("staging:board: remove unnecessary function")
>> fixed the build of drivers/staging/board/board.c.
>>
>> Signed-off-by: Geert Uytterhoeven <[email protected]>
>
> Signed-off-by: Simon Horman <[email protected]>

Acked-by? Reviewed-by? Tested-by? Rejected-by?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2015-04-08 00:44:16

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH 1/6] Revert "staging: board: disable as it breaks the build"

On Tue, Apr 07, 2015 at 03:16:54PM +0200, Geert Uytterhoeven wrote:
> On Mon, Apr 6, 2015 at 2:40 AM, Simon Horman <[email protected]> wrote:
> > On Fri, Apr 03, 2015 at 02:41:58PM +0200, Geert Uytterhoeven wrote:
> >> This reverts commit d13778d537a0ed6115d2a79a942af999cfb8eec6.
> >>
> >> Commit 13c11072536f2613 ("staging:board: remove unnecessary function")
> >> fixed the build of drivers/staging/board/board.c.
> >>
> >> Signed-off-by: Geert Uytterhoeven <[email protected]>
> >
> > Signed-off-by: Simon Horman <[email protected]>
>
> Acked-by? Reviewed-by? Tested-by? Rejected-by?

To be honest I was flipping between Signed-off-by and Acked-by.
But it seems you don't think either of those would be appropriate.
So how about:

Reviewed-by: Simon Horman <[email protected]>