linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/5] MIPS: Add virtual Ranchu board as a generic-based board
@ 2017-10-30 11:56 Aleksandar Markovic
  2017-10-30 11:56 ` [PATCH v6 1/5] Documentation: Add device tree binding for Goldfish PIC driver Aleksandar Markovic
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Aleksandar Markovic @ 2017-10-30 11:56 UTC (permalink / raw)
  To: linux-mips
  Cc: Aleksandar Markovic, David S. Miller, Douglas Leung,
	Goran Ferenc, Greg Kroah-Hartman, James Hogan, linux-kernel,
	Mauro Carvalho Chehab, Miodrag Dinic, Miodrag Dinic, Paul Burton,
	Paul Burton, Petar Jovanovic, Raghu Gandham, Ralf Baechle,
	Randy Dunlap

From: Aleksandar Markovic <aleksandar.markovic@mips.com>

v5->v6:

    - revised cascading handling code in Goldfish PIC implementation
    - used more generic node name in Goldfish PIC documentation file
    - used more generic node name in Goldfish FB documentation file
    - corrected several minor items in both documentation files
    - revisited copyright messages in two source files
    - rebased to the latest code

v4->v5:

    - removed RTC clock-related patches since they are already applied
    - removed 8042-related patch since this issue is expected to be
      resolved on the whole platform level
    - redesigned Goldfish PIC driver
    - updated email adresses in commit messages and MAINTAINERS file
      to contain "@mips.com" instead of "@imgtec.com"
    - used "MIPS" instead of "Mips" in commit messages
    - rebased to the latest code

v3->v4:

    - corrected RTC clock patch so that it does not cause build
      errors for some targets, and limited compile support to MIPS
      architecture, since it is the only case where this driver is
      used
    - changed titles of patches 2 and 4 to make them consistent
      with commit messages of corresponding directories
    - applied "checkpatch --strict" to the whole series and
      corrected several instances of reported warnings
    - rebased to the latest code

v2->v3:

    - fixed configuration dependency for VIRTIO_NET and
        RTC_DRV_GOLDFISH
    - fixed frequency calculation in ranchu_measure_hpt_freq()
    - use DT info instead of hard-coding RTC base in
        ranchu_measure_hpt_freq()
    - Goldfish PIC reworked to follow legacy irq domain paradigm
    - Goldfish RTC reimplemented to support alarm functionality
    - added COMPILE_TEST to Goldfish PIC & RTC to extend compile
        test coverage
    - corrected location of documentation for Goldfish FB
    - added a patch on unselecting ARCH_MIGHT_HAVE_PC_SERIO
    - removed two patches on i8042 as not needed in new organization
    - removed the patch on separate MIPS Android config as not needed
    - rebased to the latest code

v1->v2:

    - patch on RTC driver cleaned up
    - added drivers for virtio console and net to the Ranchu board
    - minor improvements in commit messages
    - updated recipient lists using get_maintainer.pl
    - rebased to the latest code

This series adds MIPS Ranchu virtual machine used by Android emulator.
The board relies on the concept of MIPS generic boards, and utilizes
generic board framework for build and device organization.

The Ranchu board is intended to be used by Android emulator.The name
"Ranchu" originates from Android development community. "Goldfish" and
"Ranchu" are names for two generations of virtual boards used by
Android emulator. "Ranchu" is a newer one among the two, and this
series deals with Ranchu. However, for historical reasons, some file,
device, and variable names in this series still contain the word
"Goldfish".

MIPS Ranchu machine includes a number of Goldfish devices. The
support for Virtio devices is also included. Ranchu board supports
up to 16 virtio devices which can be attached using virtio MMIO Bus.
This is summarized in the following picture:

       ABUS
        ||----MIPS CPU
        ||       |                    IRQs
        ||----Goldfish PIC------------(32)--------
        ||                     | | | | | | | | |
        ||----Goldfish TTY------ | | | | | | | |
        ||                       | | | | | | | |
        ||----Goldfish RTC-------- | | | | | | |
        ||                         | | | | | | |
        ||----Goldfish FB----------- | | | | | |
        ||                           | | | | | |
        ||----Goldfish Events--------- | | | | |
        ||                             | | | | |
        ||----Goldfish Audio------------ | | | |
        ||                               | | | |
        ||----Goldfish Battery------------ | | |
        ||                                 | | |
        ||----Android PIPE------------------ | |
        ||                                   | |
        ||----Virtio MMIO Bus                | |
        ||    |    |    |                    | |
        ||    |    |   (virtio-block)--------- |
        ||   (16)  |                           |
        ||    |   (virtio-net)------------------


Device Tree is created on the QEMU side based on the information about
devices IO map and IRQ numbers. Kernel will load this DTB using UHI
boot protocol.

Checkpatch script outputs a small number of warnings if applied to
this series. We did not correct the code, since we think the code is
correct for those particular cases of checkpatch warnings.

Aleksandar Markovic (2):
  Documentation: Add device tree binding for Goldfish FB driver
  video: goldfishfb: Add support for device tree bindings

Miodrag Dinic (3):
  Documentation: Add device tree binding for Goldfish PIC driver
  irqchip/irq-goldfish-pic: Add Goldfish PIC driver
  MIPS: ranchu: Add Ranchu as a new generic-based board

 .../bindings/display/google,goldfish-fb.txt        |  17 +++
 .../interrupt-controller/google,goldfish-pic.txt   |  30 +++++
 MAINTAINERS                                        |  12 ++
 arch/mips/configs/generic/board-ranchu.config      |  30 +++++
 arch/mips/generic/Kconfig                          |  10 ++
 arch/mips/generic/Makefile                         |   1 +
 arch/mips/generic/board-ranchu.c                   |  79 +++++++++++++
 drivers/irqchip/Kconfig                            |   8 ++
 drivers/irqchip/Makefile                           |   1 +
 drivers/irqchip/irq-goldfish-pic.c                 | 128 +++++++++++++++++++++
 drivers/video/fbdev/goldfishfb.c                   |   8 +-
 11 files changed, 323 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/display/google,goldfish-fb.txt
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/google,goldfish-pic.txt
 create mode 100644 arch/mips/configs/generic/board-ranchu.config
 create mode 100644 arch/mips/generic/board-ranchu.c
 create mode 100644 drivers/irqchip/irq-goldfish-pic.c

-- 
2.7.4

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

* [PATCH v6 1/5] Documentation: Add device tree binding for Goldfish PIC driver
  2017-10-30 11:56 [PATCH v6 0/5] MIPS: Add virtual Ranchu board as a generic-based board Aleksandar Markovic
@ 2017-10-30 11:56 ` Aleksandar Markovic
  2017-10-30 11:56 ` [PATCH v6 2/5] irqchip/irq-goldfish-pic: Add " Aleksandar Markovic
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Aleksandar Markovic @ 2017-10-30 11:56 UTC (permalink / raw)
  To: linux-mips
  Cc: Miodrag Dinic, Goran Ferenc, Aleksandar Markovic,
	David S. Miller, devicetree, Douglas Leung, Greg Kroah-Hartman,
	James Hogan, Jason Cooper, linux-kernel, Marc Zyngier,
	Mark Rutland, Mauro Carvalho Chehab, Miodrag Dinic, Paul Burton,
	Petar Jovanovic, Raghu Gandham, Randy Dunlap, Rob Herring,
	Thomas Gleixner

From: Miodrag Dinic <miodrag.dinic@mips.com>

Add documentation for DT binding of Goldfish PIC driver. The compatible
string used by OS for binding the driver is "google,goldfish-pic".

Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
Signed-off-by: Goran Ferenc <goran.ferenc@mips.com>
Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 .../interrupt-controller/google,goldfish-pic.txt   | 30 ++++++++++++++++++++++
 MAINTAINERS                                        |  5 ++++
 2 files changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/google,goldfish-pic.txt

diff --git a/Documentation/devicetree/bindings/interrupt-controller/google,goldfish-pic.txt b/Documentation/devicetree/bindings/interrupt-controller/google,goldfish-pic.txt
new file mode 100644
index 0000000..35f7527
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/google,goldfish-pic.txt
@@ -0,0 +1,30 @@
+Android Goldfish PIC
+
+Android Goldfish programmable interrupt device used by Android
+emulator.
+
+Required properties:
+
+- compatible : should contain "google,goldfish-pic"
+- reg        : <registers mapping>
+- interrupts : <interrupt mapping>
+
+Example for mips when used in cascade mode:
+
+        cpuintc {
+                #interrupt-cells = <0x1>;
+                #address-cells = <0>;
+                interrupt-controller;
+                compatible = "mti,cpu-interrupt-controller";
+        };
+
+        interrupt-controller@1f000000 {
+                compatible = "google,goldfish-pic";
+                reg = <0x1f000000 0x1000>;
+
+                interrupt-controller;
+                #interrupt-cells = <0x1>;
+
+                interrupt-parent = <&cpuintc>;
+                interrupts = <0x2>;
+        };
diff --git a/MAINTAINERS b/MAINTAINERS
index 4a3de82..4d5108f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -872,6 +872,11 @@ S:	Supported
 F:	drivers/android/
 F:	drivers/staging/android/
 
+ANDROID GOLDFISH PIC DRIVER
+M:	Miodrag Dinic <miodrag.dinic@mips.com>
+S:	Supported
+F:	Documentation/devicetree/bindings/interrupt-controller/google,goldfish-pic.txt
+
 ANDROID GOLDFISH RTC DRIVER
 M:	Miodrag Dinic <miodrag.dinic@mips.com>
 S:	Supported
-- 
2.7.4

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

* [PATCH v6 2/5] irqchip/irq-goldfish-pic: Add Goldfish PIC driver
  2017-10-30 11:56 [PATCH v6 0/5] MIPS: Add virtual Ranchu board as a generic-based board Aleksandar Markovic
  2017-10-30 11:56 ` [PATCH v6 1/5] Documentation: Add device tree binding for Goldfish PIC driver Aleksandar Markovic
@ 2017-10-30 11:56 ` Aleksandar Markovic
  2017-10-31  2:26   ` Marc Zyngier
  2017-10-30 11:56 ` [PATCH v6 3/5] Documentation: Add device tree binding for Goldfish FB driver Aleksandar Markovic
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Aleksandar Markovic @ 2017-10-30 11:56 UTC (permalink / raw)
  To: linux-mips
  Cc: Miodrag Dinic, Goran Ferenc, Aleksandar Markovic,
	David S. Miller, Douglas Leung, Greg Kroah-Hartman, James Hogan,
	Jason Cooper, linux-kernel, Marc Zyngier, Mauro Carvalho Chehab,
	Miodrag Dinic, Paul Burton, Petar Jovanovic, Raghu Gandham,
	Randy Dunlap, Thomas Gleixner

From: Miodrag Dinic <miodrag.dinic@mips.com>

Add device driver for a virtual programmable interrupt controller

The virtual PIC is designed as a device tree-based interrupt controller.

The compatible string used by OS for binding the driver is
"google,goldfish-pic".

Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
Signed-off-by: Goran Ferenc <goran.ferenc@mips.com>
Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
---
 MAINTAINERS                        |   1 +
 drivers/irqchip/Kconfig            |   8 +++
 drivers/irqchip/Makefile           |   1 +
 drivers/irqchip/irq-goldfish-pic.c | 128 +++++++++++++++++++++++++++++++++++++
 4 files changed, 138 insertions(+)
 create mode 100644 drivers/irqchip/irq-goldfish-pic.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 4d5108f..f1be016 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -876,6 +876,7 @@ ANDROID GOLDFISH PIC DRIVER
 M:	Miodrag Dinic <miodrag.dinic@mips.com>
 S:	Supported
 F:	Documentation/devicetree/bindings/interrupt-controller/google,goldfish-pic.txt
+F:	drivers/irqchip/irq-goldfish-pic.c
 
 ANDROID GOLDFISH RTC DRIVER
 M:	Miodrag Dinic <miodrag.dinic@mips.com>
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 9d8a1dd..712b561 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -321,3 +321,11 @@ config IRQ_UNIPHIER_AIDET
 	select IRQ_DOMAIN_HIERARCHY
 	help
 	  Support for the UniPhier AIDET (ARM Interrupt Detector).
+
+config GOLDFISH_PIC
+	bool "Goldfish programmable interrupt controller"
+	depends on MIPS && (GOLDFISH || COMPILE_TEST)
+	select IRQ_DOMAIN
+	help
+	  Say yes here to enable Goldfish interrupt controller driver used
+	  for Goldfish based virtual platforms.
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 845abc1..0e7a224 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -79,3 +79,4 @@ obj-$(CONFIG_ARCH_ASPEED)		+= irq-aspeed-vic.o irq-aspeed-i2c-ic.o
 obj-$(CONFIG_STM32_EXTI) 		+= irq-stm32-exti.o
 obj-$(CONFIG_QCOM_IRQ_COMBINER)		+= qcom-irq-combiner.o
 obj-$(CONFIG_IRQ_UNIPHIER_AIDET)	+= irq-uniphier-aidet.o
+obj-$(CONFIG_GOLDFISH_PIC) 		+= irq-goldfish-pic.o
diff --git a/drivers/irqchip/irq-goldfish-pic.c b/drivers/irqchip/irq-goldfish-pic.c
new file mode 100644
index 0000000..48fb773
--- /dev/null
+++ b/drivers/irqchip/irq-goldfish-pic.c
@@ -0,0 +1,128 @@
+/*
+ * Driver for MIPS Goldfish Programmable Interrupt Controller.
+ *
+ * Author: Miodrag Dinic <miodrag.dinic@mips.com>
+ *
+ * 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;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+
+#define GFPIC_NR_IRQS			32
+
+/* 8..39 Cascaded Goldfish PIC interrupts */
+#define GFPIC_IRQ_BASE			8
+
+#define GFPIC_REG_IRQ_PENDING		0x04
+#define GFPIC_REG_IRQ_DISABLE_ALL	0x08
+#define GFPIC_REG_IRQ_DISABLE		0x0c
+#define GFPIC_REG_IRQ_ENABLE		0x10
+
+struct goldfish_pic_data {
+	void __iomem *base;
+	struct irq_domain *irq_domain;
+};
+
+static void goldfish_pic_cascade(struct irq_desc *desc)
+{
+	struct goldfish_pic_data *gfpic = irq_desc_get_handler_data(desc);
+	struct irq_chip *host_chip = irq_desc_get_chip(desc);
+	u32 pending, hwirq, virq;
+
+	chained_irq_enter(host_chip, desc);
+
+	pending = readl(gfpic->base + GFPIC_REG_IRQ_PENDING);
+	while (pending) {
+		hwirq = __fls(pending);
+		virq = irq_linear_revmap(gfpic->irq_domain, hwirq);
+		generic_handle_irq(virq);
+		pending &= ~(1 << hwirq);
+	}
+
+	chained_irq_exit(host_chip, desc);
+}
+
+static const struct irq_domain_ops goldfish_irq_domain_ops = {
+	.xlate = irq_domain_xlate_onecell,
+};
+
+static int __init goldfish_pic_of_init(struct device_node *of_node,
+				       struct device_node *parent)
+{
+	struct goldfish_pic_data *gfpic;
+	struct irq_chip_generic *gc;
+	struct irq_chip_type *ct;
+	unsigned int parent_irq;
+	int ret = 0;
+
+	gfpic = kzalloc(sizeof(*gfpic), GFP_KERNEL);
+	if (!gfpic) {
+		ret = -ENOMEM;
+		goto out_err;
+	}
+
+	parent_irq = irq_of_parse_and_map(of_node, 0);
+	if (!parent_irq) {
+		pr_err("Failed to map Goldfish PIC parent IRQ\n");
+		ret = -EINVAL;
+		goto out_free;
+	}
+
+	gfpic->base = of_iomap(of_node, 0);
+	if (!gfpic->base) {
+		pr_err("Failed to map Goldfish PIC base\n");
+		ret = -ENOMEM;
+		goto out_unmap_irq;
+	}
+
+	/* Mask interrupts. */
+	writel(1, gfpic->base + GFPIC_REG_IRQ_DISABLE_ALL);
+
+	gc = irq_alloc_generic_chip("GFPIC", 1, GFPIC_IRQ_BASE, gfpic->base,
+				    handle_level_irq);
+
+	ct = gc->chip_types;
+	ct->regs.enable = GFPIC_REG_IRQ_ENABLE;
+	ct->regs.disable = GFPIC_REG_IRQ_DISABLE;
+	ct->chip.irq_unmask = irq_gc_unmask_enable_reg;
+	ct->chip.irq_mask = irq_gc_mask_disable_reg;
+
+	irq_setup_generic_chip(gc, IRQ_MSK(GFPIC_NR_IRQS), 0, 0,
+			       IRQ_NOPROBE | IRQ_LEVEL);
+
+	gfpic->irq_domain = irq_domain_add_legacy(of_node, GFPIC_NR_IRQS,
+						  GFPIC_IRQ_BASE, 0,
+						  &goldfish_irq_domain_ops,
+						  NULL);
+	if (!gfpic->irq_domain) {
+		pr_err("Failed to add irqdomain for Goldfish PIC\n");
+		ret = -EINVAL;
+		goto out_iounmap;
+	}
+
+	irq_set_chained_handler_and_data(parent_irq,
+					 goldfish_pic_cascade, gfpic);
+
+	pr_info("Successfully registered Goldfish PIC\n");
+	return 0;
+
+out_iounmap:
+	iounmap(gfpic->base);
+out_unmap_irq:
+	irq_dispose_mapping(parent_irq);
+out_free:
+	kfree(gfpic);
+out_err:
+	return ret;
+}
+
+IRQCHIP_DECLARE(google_gf_pic, "google,goldfish-pic", goldfish_pic_of_init);
-- 
2.7.4

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

* [PATCH v6 3/5] Documentation: Add device tree binding for Goldfish FB driver
  2017-10-30 11:56 [PATCH v6 0/5] MIPS: Add virtual Ranchu board as a generic-based board Aleksandar Markovic
  2017-10-30 11:56 ` [PATCH v6 1/5] Documentation: Add device tree binding for Goldfish PIC driver Aleksandar Markovic
  2017-10-30 11:56 ` [PATCH v6 2/5] irqchip/irq-goldfish-pic: Add " Aleksandar Markovic
@ 2017-10-30 11:56 ` Aleksandar Markovic
  2017-10-30 11:56 ` [PATCH v6 4/5] video: goldfishfb: Add support for device tree bindings Aleksandar Markovic
  2017-10-30 11:56 ` [PATCH v6 5/5] MIPS: ranchu: Add Ranchu as a new generic-based board Aleksandar Markovic
  4 siblings, 0 replies; 15+ messages in thread
From: Aleksandar Markovic @ 2017-10-30 11:56 UTC (permalink / raw)
  To: linux-mips
  Cc: Aleksandar Markovic, Miodrag Dinic, Goran Ferenc, David Airlie,
	devicetree, Douglas Leung, dri-devel, James Hogan, linux-kernel,
	Mark Rutland, Paul Burton, Petar Jovanovic, Raghu Gandham,
	Rob Herring

From: Aleksandar Markovic <aleksandar.markovic@mips.com>

Add documentation for DT binding of Goldfish FB driver. The compatible
string used by OS for binding the driver is "google,goldfish-fb".

Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
Signed-off-by: Goran Ferenc <goran.ferenc@mips.com>
Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/display/google,goldfish-fb.txt  | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/google,goldfish-fb.txt

diff --git a/Documentation/devicetree/bindings/display/google,goldfish-fb.txt b/Documentation/devicetree/bindings/display/google,goldfish-fb.txt
new file mode 100644
index 0000000..751fa9f
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/google,goldfish-fb.txt
@@ -0,0 +1,17 @@
+Android Goldfish framebuffer
+
+Android Goldfish framebuffer device used by Android emulator.
+
+Required properties:
+
+- compatible : should contain "google,goldfish-fb"
+- reg        : <registers mapping>
+- interrupts : <interrupt mapping>
+
+Example:
+
+	display-controller@1f008000 {
+		compatible = "google,goldfish-fb";
+		interrupts = <0x10>;
+		reg = <0x1f008000 0x100>;
+	};
-- 
2.7.4

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

* [PATCH v6 4/5] video: goldfishfb: Add support for device tree bindings
  2017-10-30 11:56 [PATCH v6 0/5] MIPS: Add virtual Ranchu board as a generic-based board Aleksandar Markovic
                   ` (2 preceding siblings ...)
  2017-10-30 11:56 ` [PATCH v6 3/5] Documentation: Add device tree binding for Goldfish FB driver Aleksandar Markovic
@ 2017-10-30 11:56 ` Aleksandar Markovic
  2017-10-30 11:56 ` [PATCH v6 5/5] MIPS: ranchu: Add Ranchu as a new generic-based board Aleksandar Markovic
  4 siblings, 0 replies; 15+ messages in thread
From: Aleksandar Markovic @ 2017-10-30 11:56 UTC (permalink / raw)
  To: linux-mips
  Cc: Aleksandar Markovic, Miodrag Dinic, Goran Ferenc,
	Bartlomiej Zolnierkiewicz, Douglas Leung, James Hogan,
	linux-fbdev, linux-kernel, Miodrag Dinic, Paul Burton,
	Petar Jovanovic, Raghu Gandham

From: Aleksandar Markovic <aleksandar.markovic@mips.com>

Add ability to the Goldfish FB driver to be recognized by OS via DT.

Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
Signed-off-by: Goran Ferenc <goran.ferenc@mips.com>
Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
---
 drivers/video/fbdev/goldfishfb.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/goldfishfb.c b/drivers/video/fbdev/goldfishfb.c
index 7f6c9e6..3b70044 100644
--- a/drivers/video/fbdev/goldfishfb.c
+++ b/drivers/video/fbdev/goldfishfb.c
@@ -304,12 +304,18 @@ static int goldfish_fb_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id goldfish_fb_of_match[] = {
+	{ .compatible = "google,goldfish-fb", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, goldfish_fb_of_match);
 
 static struct platform_driver goldfish_fb_driver = {
 	.probe		= goldfish_fb_probe,
 	.remove		= goldfish_fb_remove,
 	.driver = {
-		.name = "goldfish_fb"
+		.name = "goldfish_fb",
+		.of_match_table = goldfish_fb_of_match,
 	}
 };
 
-- 
2.7.4

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

* [PATCH v6 5/5] MIPS: ranchu: Add Ranchu as a new generic-based board
  2017-10-30 11:56 [PATCH v6 0/5] MIPS: Add virtual Ranchu board as a generic-based board Aleksandar Markovic
                   ` (3 preceding siblings ...)
  2017-10-30 11:56 ` [PATCH v6 4/5] video: goldfishfb: Add support for device tree bindings Aleksandar Markovic
@ 2017-10-30 11:56 ` Aleksandar Markovic
  2017-10-30 16:45   ` James Hogan
  2017-11-01 17:58   ` Paul Burton
  4 siblings, 2 replies; 15+ messages in thread
From: Aleksandar Markovic @ 2017-10-30 11:56 UTC (permalink / raw)
  To: linux-mips
  Cc: Miodrag Dinic, Goran Ferenc, Aleksandar Markovic,
	David S. Miller, Douglas Leung, Greg Kroah-Hartman, James Hogan,
	linux-kernel, Mauro Carvalho Chehab, Miodrag Dinic, Paul Burton,
	Paul Burton, Petar Jovanovic, Raghu Gandham, Ralf Baechle,
	Randy Dunlap

From: Miodrag Dinic <miodrag.dinic@mips.com>

Provide amendments to the MIPS generic platform framework so that
the new generic-based board Ranchu can be chosen to be built.

Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
Signed-off-by: Goran Ferenc <goran.ferenc@mips.com>
Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
---
 MAINTAINERS                                   |  6 ++
 arch/mips/configs/generic/board-ranchu.config | 30 ++++++++++
 arch/mips/generic/Kconfig                     | 10 ++++
 arch/mips/generic/Makefile                    |  1 +
 arch/mips/generic/board-ranchu.c              | 79 +++++++++++++++++++++++++++
 5 files changed, 126 insertions(+)
 create mode 100644 arch/mips/configs/generic/board-ranchu.config
 create mode 100644 arch/mips/generic/board-ranchu.c

diff --git a/MAINTAINERS b/MAINTAINERS
index f1be016..e429cc2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11308,6 +11308,12 @@ S:	Maintained
 F:	Documentation/blockdev/ramdisk.txt
 F:	drivers/block/brd.c
 
+RANCHU VIRTUAL BOARD FOR MIPS
+M:	Miodrag Dinic <miodrag.dinic@mips.com>
+L:	linux-mips@linux-mips.org
+S:	Supported
+F:	arch/mips/generic/board-ranchu.c
+
 RANDOM NUMBER DRIVER
 M:	"Theodore Ts'o" <tytso@mit.edu>
 S:	Maintained
diff --git a/arch/mips/configs/generic/board-ranchu.config b/arch/mips/configs/generic/board-ranchu.config
new file mode 100644
index 0000000..fee9ad4
--- /dev/null
+++ b/arch/mips/configs/generic/board-ranchu.config
@@ -0,0 +1,30 @@
+CONFIG_VIRT_BOARD_RANCHU=y
+
+CONFIG_BATTERY_GOLDFISH=y
+CONFIG_FB=y
+CONFIG_FB_GOLDFISH=y
+CONFIG_GOLDFISH=y
+CONFIG_STAGING=y
+CONFIG_GOLDFISH_AUDIO=y
+CONFIG_GOLDFISH_PIC=y
+CONFIG_GOLDFISH_PIPE=y
+CONFIG_GOLDFISH_TTY=y
+CONFIG_RTC_CLASS=y
+CONFIG_RTC_DRV_GOLDFISH=y
+
+CONFIG_INPUT_EVDEV=y
+CONFIG_INPUT_KEYBOARD=y
+CONFIG_KEYBOARD_GOLDFISH_EVENTS=y
+
+CONFIG_MAGIC_SYSRQ=y
+CONFIG_POWER_SUPPLY=y
+CONFIG_POWER_RESET=y
+CONFIG_POWER_RESET_SYSCON=y
+CONFIG_POWER_RESET_SYSCON_POWEROFF=y
+
+CONFIG_VIRTIO_BLK=y
+CONFIG_VIRTIO_CONSOLE=y
+CONFIG_VIRTIO_MMIO=y
+CONFIG_VIRTIO_MMIO_CMDLINE_DEVICES=y
+CONFIG_NETDEVICES=y
+CONFIG_VIRTIO_NET=y
diff --git a/arch/mips/generic/Kconfig b/arch/mips/generic/Kconfig
index e0436aa..93582be 100644
--- a/arch/mips/generic/Kconfig
+++ b/arch/mips/generic/Kconfig
@@ -42,4 +42,14 @@ config FIT_IMAGE_FDT_NI169445
 	  Enable this to include the FDT for the 169445 platform from
 	  National Instruments in the FIT kernel image.
 
+config VIRT_BOARD_RANCHU
+	bool "Ranchu platform for Android emulator"
+	help
+	  This enables support for the platform used by Android emulator.
+
+	  Ranchu platform consists of a set of virtual devices. This platform
+	  enables emulation of variety of virtual configurations while using
+	  Android emulator. Android emulator is based on Qemu, and contains
+	  the support for the same set of virtual devices.
+
 endif
diff --git a/arch/mips/generic/Makefile b/arch/mips/generic/Makefile
index 56b3ea5..2fee84a 100644
--- a/arch/mips/generic/Makefile
+++ b/arch/mips/generic/Makefile
@@ -15,3 +15,4 @@ obj-y += proc.o
 obj-$(CONFIG_YAMON_DT_SHIM)		+= yamon-dt.o
 obj-$(CONFIG_LEGACY_BOARD_SEAD3)	+= board-sead3.o
 obj-$(CONFIG_KEXEC)			+= kexec.o
+obj-$(CONFIG_VIRT_BOARD_RANCHU)	+= board-ranchu.o
diff --git a/arch/mips/generic/board-ranchu.c b/arch/mips/generic/board-ranchu.c
new file mode 100644
index 0000000..0397752
--- /dev/null
+++ b/arch/mips/generic/board-ranchu.c
@@ -0,0 +1,79 @@
+/*
+ * Support code for virtual Ranchu board for MIPS.
+ *
+ * Author: Miodrag Dinic <miodrag.dinic@mips.com>
+ *
+ * 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;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+#include <linux/of_address.h>
+
+#include <asm/machine.h>
+#include <asm/time.h>
+
+#define GOLDFISH_TIMER_LOW		0x00
+#define GOLDFISH_TIMER_HIGH		0x04
+
+static __init uint64_t read_rtc_time(void __iomem *base)
+{
+	u64 time_low;
+	u64 time_high;
+
+	time_low = readl(base + GOLDFISH_TIMER_LOW);
+	time_high = readl(base + GOLDFISH_TIMER_HIGH);
+
+	return (time_high << 32) | time_low;
+}
+
+static __init unsigned int ranchu_measure_hpt_freq(void)
+{
+	u64 rtc_start, rtc_current, rtc_delta;
+	unsigned int start, count;
+	struct device_node *np;
+	void __iomem *rtc_base;
+
+	np = of_find_compatible_node(NULL, NULL, "google,goldfish-rtc");
+	if (!np)
+		panic("%s(): Failed to find 'google,goldfish-rtc' dt node!",
+		      __func__);
+
+	rtc_base = of_iomap(np, 0);
+	if (!rtc_base)
+		panic("%s(): Failed to ioremap Goldfish RTC base!", __func__);
+
+	/*
+	 * poll the nanosecond resolution RTC for 1 second
+	 * to calibrate the CPU frequency
+	 */
+	rtc_start = read_rtc_time(rtc_base);
+	start = read_c0_count();
+
+	do {
+		rtc_current = read_rtc_time(rtc_base);
+		rtc_delta = rtc_current - rtc_start;
+	} while (rtc_delta < NSEC_PER_SEC);
+
+	count = read_c0_count() - start;
+
+	count += 5000;	/* round */
+	count -= count % 10000;
+
+	return count;
+}
+
+static const struct of_device_id ranchu_of_match[];
+
+MIPS_MACHINE(ranchu) = {
+	.matches = ranchu_of_match,
+	.measure_hpt_freq = ranchu_measure_hpt_freq,
+};
+
+static const struct of_device_id ranchu_of_match[] = {
+	{
+		.compatible = "mti,ranchu",
+		.data = &__mips_mach_ranchu,
+	},
+};
-- 
2.7.4

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

* Re: [PATCH v6 5/5] MIPS: ranchu: Add Ranchu as a new generic-based board
  2017-10-30 11:56 ` [PATCH v6 5/5] MIPS: ranchu: Add Ranchu as a new generic-based board Aleksandar Markovic
@ 2017-10-30 16:45   ` James Hogan
  2017-11-02 12:47     ` Miodrag Dinic
  2017-11-01 17:58   ` Paul Burton
  1 sibling, 1 reply; 15+ messages in thread
From: James Hogan @ 2017-10-30 16:45 UTC (permalink / raw)
  To: Aleksandar Markovic
  Cc: linux-mips, Miodrag Dinic, Goran Ferenc, Aleksandar Markovic,
	David S. Miller, Douglas Leung, Greg Kroah-Hartman, linux-kernel,
	Mauro Carvalho Chehab, Miodrag Dinic, Paul Burton, Paul Burton,
	Petar Jovanovic, Raghu Gandham, Ralf Baechle, Randy Dunlap

On Mon, Oct 30, 2017 at 12:56:36PM +0100, Aleksandar Markovic wrote:
> From: Miodrag Dinic <miodrag.dinic@mips.com>
> 
> Provide amendments to the MIPS generic platform framework so that
> the new generic-based board Ranchu can be chosen to be built.

A bit more info about the board would be good here. What boot protocol
is used? Does QEMU generate the DT dynamically?

> 
> Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
> Signed-off-by: Goran Ferenc <goran.ferenc@mips.com>
> Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
> ---
>  MAINTAINERS                                   |  6 ++
>  arch/mips/configs/generic/board-ranchu.config | 30 ++++++++++
>  arch/mips/generic/Kconfig                     | 10 ++++
>  arch/mips/generic/Makefile                    |  1 +
>  arch/mips/generic/board-ranchu.c              | 79 +++++++++++++++++++++++++++
>  5 files changed, 126 insertions(+)
>  create mode 100644 arch/mips/configs/generic/board-ranchu.config
>  create mode 100644 arch/mips/generic/board-ranchu.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f1be016..e429cc2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11308,6 +11308,12 @@ S:	Maintained
>  F:	Documentation/blockdev/ramdisk.txt
>  F:	drivers/block/brd.c
>  
> +RANCHU VIRTUAL BOARD FOR MIPS
> +M:	Miodrag Dinic <miodrag.dinic@mips.com>
> +L:	linux-mips@linux-mips.org
> +S:	Supported
> +F:	arch/mips/generic/board-ranchu.c

Maybe worth adding arch/mips/configs/generic/board-ranchu.config too.

> +
>  RANDOM NUMBER DRIVER
>  M:	"Theodore Ts'o" <tytso@mit.edu>
>  S:	Maintained

> diff --git a/arch/mips/generic/Kconfig b/arch/mips/generic/Kconfig
> index e0436aa..93582be 100644
> --- a/arch/mips/generic/Kconfig
> +++ b/arch/mips/generic/Kconfig
> @@ -42,4 +42,14 @@ config FIT_IMAGE_FDT_NI169445
>  	  Enable this to include the FDT for the 169445 platform from
>  	  National Instruments in the FIT kernel image.
>  
> +config VIRT_BOARD_RANCHU
> +	bool "Ranchu platform for Android emulator"
> +	help
> +	  This enables support for the platform used by Android emulator.
> +
> +	  Ranchu platform consists of a set of virtual devices. This platform
> +	  enables emulation of variety of virtual configurations while using
> +	  Android emulator. Android emulator is based on Qemu, and contains
> +	  the support for the same set of virtual devices.

This is effectively in the section "FIT/UHI Boards", but it has a
platform file and no DT/FIT stuff in tree a bit like the boards in the
section "Legacy (non-UHI/non-FIT) Boards".

I'm guessing it might be something in between, with UHI + platform code,
but DT provided by QEMU (i.e. FIT support makes no sense)?

If it uses UHI I suppose it doesn't belong in the legacy section, but I
think a consistent prompt would be beneficial, e.g.

+config VIRT_BOARD_RANCHU
+	bool "Support Ranchu platform for Android emulator"
...

> diff --git a/arch/mips/generic/board-ranchu.c b/arch/mips/generic/board-ranchu.c
> new file mode 100644
> index 0000000..0397752
> --- /dev/null
> +++ b/arch/mips/generic/board-ranchu.c
> @@ -0,0 +1,79 @@
> +/*
> + * Support code for virtual Ranchu board for MIPS.
> + *
> + * Author: Miodrag Dinic <miodrag.dinic@mips.com>
> + *
> + * 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;  either version 2 of the  License, or (at your
> + * option) any later version.
> + */
> +
> +#include <linux/of_address.h>
> +
> +#include <asm/machine.h>
> +#include <asm/time.h>
> +
> +#define GOLDFISH_TIMER_LOW		0x00
> +#define GOLDFISH_TIMER_HIGH		0x04
> +
> +static __init uint64_t read_rtc_time(void __iomem *base)
> +{
> +	u64 time_low;
> +	u64 time_high;
> +
> +	time_low = readl(base + GOLDFISH_TIMER_LOW);
> +	time_high = readl(base + GOLDFISH_TIMER_HIGH);
> +
> +	return (time_high << 32) | time_low;

What if high changes while reading this?

E.g.
TIMER_LOW	 0x00000000 *0xffffffff*
TIMER_HIGH	*0x00000001* 0x00000000

You'd presumably get 0x00000001ffffffff.

Perhaps it should read HIGH before too, and retry if it has changed.

Cheers
James

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

* Re: [PATCH v6 2/5] irqchip/irq-goldfish-pic: Add Goldfish PIC driver
  2017-10-30 11:56 ` [PATCH v6 2/5] irqchip/irq-goldfish-pic: Add " Aleksandar Markovic
@ 2017-10-31  2:26   ` Marc Zyngier
  2017-11-01 14:34     ` Miodrag Dinic
  0 siblings, 1 reply; 15+ messages in thread
From: Marc Zyngier @ 2017-10-31  2:26 UTC (permalink / raw)
  To: Aleksandar Markovic
  Cc: linux-mips, Miodrag Dinic, Goran Ferenc, Aleksandar Markovic,
	David S. Miller, Douglas Leung, Greg Kroah-Hartman, James Hogan,
	Jason Cooper, linux-kernel, Mauro Carvalho Chehab, Miodrag Dinic,
	Paul Burton, Petar Jovanovic, Raghu Gandham, Randy Dunlap,
	Thomas Gleixner

On Mon, Oct 30 2017 at 12:56:33 pm GMT, Aleksandar Markovic <aleksandar.markovic@rt-rk.com> wrote:
> From: Miodrag Dinic <miodrag.dinic@mips.com>
>
> Add device driver for a virtual programmable interrupt controller
>
> The virtual PIC is designed as a device tree-based interrupt controller.
>
> The compatible string used by OS for binding the driver is
> "google,goldfish-pic".
>
> Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
> Signed-off-by: Goran Ferenc <goran.ferenc@mips.com>
> Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>

[...]

> diff --git a/drivers/irqchip/irq-goldfish-pic.c b/drivers/irqchip/irq-goldfish-pic.c
> new file mode 100644
> index 0000000..48fb773
> --- /dev/null
> +++ b/drivers/irqchip/irq-goldfish-pic.c

[...]

> +static int __init goldfish_pic_of_init(struct device_node *of_node,
> +				       struct device_node *parent)
> +{
> +	struct goldfish_pic_data *gfpic;
> +	struct irq_chip_generic *gc;
> +	struct irq_chip_type *ct;
> +	unsigned int parent_irq;
> +	int ret = 0;
> +
> +	gfpic = kzalloc(sizeof(*gfpic), GFP_KERNEL);
> +	if (!gfpic) {
> +		ret = -ENOMEM;
> +		goto out_err;
> +	}
> +
> +	parent_irq = irq_of_parse_and_map(of_node, 0);
> +	if (!parent_irq) {
> +		pr_err("Failed to map Goldfish PIC parent IRQ\n");
> +		ret = -EINVAL;
> +		goto out_free;
> +	}
> +
> +	gfpic->base = of_iomap(of_node, 0);
> +	if (!gfpic->base) {
> +		pr_err("Failed to map Goldfish PIC base\n");
> +		ret = -ENOMEM;
> +		goto out_unmap_irq;
> +	}
> +
> +	/* Mask interrupts. */
> +	writel(1, gfpic->base + GFPIC_REG_IRQ_DISABLE_ALL);
> +
> +	gc = irq_alloc_generic_chip("GFPIC", 1, GFPIC_IRQ_BASE, gfpic->base,
> +				    handle_level_irq);
> +
> +	ct = gc->chip_types;

And what if the allocation fails?

> +	ct->regs.enable = GFPIC_REG_IRQ_ENABLE;
> +	ct->regs.disable = GFPIC_REG_IRQ_DISABLE;
> +	ct->chip.irq_unmask = irq_gc_unmask_enable_reg;
> +	ct->chip.irq_mask = irq_gc_mask_disable_reg;
> +
> +	irq_setup_generic_chip(gc, IRQ_MSK(GFPIC_NR_IRQS), 0, 0,
> +			       IRQ_NOPROBE | IRQ_LEVEL);
> +
> +	gfpic->irq_domain = irq_domain_add_legacy(of_node, GFPIC_NR_IRQS,
> +						  GFPIC_IRQ_BASE, 0,
> +						  &goldfish_irq_domain_ops,
> +						  NULL);
> +	if (!gfpic->irq_domain) {
> +		pr_err("Failed to add irqdomain for Goldfish PIC\n");
> +		ret = -EINVAL;
> +		goto out_iounmap;
> +	}
> +
> +	irq_set_chained_handler_and_data(parent_irq,
> +					 goldfish_pic_cascade, gfpic);
> +
> +	pr_info("Successfully registered Goldfish PIC\n");
> +	return 0;
> +
> +out_iounmap:
> +	iounmap(gfpic->base);
> +out_unmap_irq:
> +	irq_dispose_mapping(parent_irq); 
> +out_free:
> +	kfree(gfpic);
> +out_err:
> +	return ret;
> +}
> +
> +IRQCHIP_DECLARE(google_gf_pic, "google,goldfish-pic", goldfish_pic_of_init);

Thanks,

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

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

* RE: [PATCH v6 2/5] irqchip/irq-goldfish-pic: Add Goldfish PIC driver
  2017-10-31  2:26   ` Marc Zyngier
@ 2017-11-01 14:34     ` Miodrag Dinic
  0 siblings, 0 replies; 15+ messages in thread
From: Miodrag Dinic @ 2017-11-01 14:34 UTC (permalink / raw)
  To: Marc Zyngier, Aleksandar Markovic
  Cc: linux-mips, Goran Ferenc, Aleksandar Markovic, David S. Miller,
	Douglas Leung, Greg Kroah-Hartman, James Hogan, Jason Cooper,
	linux-kernel, Mauro Carvalho Chehab, Miodrag Dinic, Paul Burton,
	Petar Jovanovic, Raghu Gandham, Randy Dunlap, Thomas Gleixner

Hello Marc,

> > +     gc = irq_alloc_generic_chip("GFPIC", 1, GFPIC_IRQ_BASE, gfpic->base,
> > +                                 handle_level_irq);
> > +
> > +     ct = gc->chip_types;
> 
> And what if the allocation fails?

Thanks, it will be addressed in V7.

Kind regards,
Miodrag
________________________________________
From: Marc Zyngier [marc.zyngier@arm.com]
Sent: Tuesday, October 31, 2017 3:26 AM
To: Aleksandar Markovic
Cc: linux-mips@linux-mips.org; Miodrag Dinic; Goran Ferenc; Aleksandar Markovic; David S. Miller; Douglas Leung; Greg Kroah-Hartman; James Hogan; Jason Cooper; linux-kernel@vger.kernel.org; Mauro Carvalho Chehab; Miodrag Dinic; Paul Burton; Petar Jovanovic; Raghu Gandham; Randy Dunlap; Thomas Gleixner
Subject: Re: [PATCH v6 2/5] irqchip/irq-goldfish-pic: Add Goldfish PIC driver

On Mon, Oct 30 2017 at 12:56:33 pm GMT, Aleksandar Markovic <aleksandar.markovic@rt-rk.com> wrote:
> From: Miodrag Dinic <miodrag.dinic@mips.com>
>
> Add device driver for a virtual programmable interrupt controller
>
> The virtual PIC is designed as a device tree-based interrupt controller.
>
> The compatible string used by OS for binding the driver is
> "google,goldfish-pic".
>
> Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
> Signed-off-by: Goran Ferenc <goran.ferenc@mips.com>
> Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>

[...]

> diff --git a/drivers/irqchip/irq-goldfish-pic.c b/drivers/irqchip/irq-goldfish-pic.c
> new file mode 100644
> index 0000000..48fb773
> --- /dev/null
> +++ b/drivers/irqchip/irq-goldfish-pic.c

[...]

> +static int __init goldfish_pic_of_init(struct device_node *of_node,
> +                                    struct device_node *parent)
> +{
> +     struct goldfish_pic_data *gfpic;
> +     struct irq_chip_generic *gc;
> +     struct irq_chip_type *ct;
> +     unsigned int parent_irq;
> +     int ret = 0;
> +
> +     gfpic = kzalloc(sizeof(*gfpic), GFP_KERNEL);
> +     if (!gfpic) {
> +             ret = -ENOMEM;
> +             goto out_err;
> +     }
> +
> +     parent_irq = irq_of_parse_and_map(of_node, 0);
> +     if (!parent_irq) {
> +             pr_err("Failed to map Goldfish PIC parent IRQ\n");
> +             ret = -EINVAL;
> +             goto out_free;
> +     }
> +
> +     gfpic->base = of_iomap(of_node, 0);
> +     if (!gfpic->base) {
> +             pr_err("Failed to map Goldfish PIC base\n");
> +             ret = -ENOMEM;
> +             goto out_unmap_irq;
> +     }
> +
> +     /* Mask interrupts. */
> +     writel(1, gfpic->base + GFPIC_REG_IRQ_DISABLE_ALL);
> +
> +     gc = irq_alloc_generic_chip("GFPIC", 1, GFPIC_IRQ_BASE, gfpic->base,
> +                                 handle_level_irq);
> +
> +     ct = gc->chip_types;

And what if the allocation fails?

> +     ct->regs.enable = GFPIC_REG_IRQ_ENABLE;
> +     ct->regs.disable = GFPIC_REG_IRQ_DISABLE;
> +     ct->chip.irq_unmask = irq_gc_unmask_enable_reg;
> +     ct->chip.irq_mask = irq_gc_mask_disable_reg;
> +
> +     irq_setup_generic_chip(gc, IRQ_MSK(GFPIC_NR_IRQS), 0, 0,
> +                            IRQ_NOPROBE | IRQ_LEVEL);
> +
> +     gfpic->irq_domain = irq_domain_add_legacy(of_node, GFPIC_NR_IRQS,
> +                                               GFPIC_IRQ_BASE, 0,
> +                                               &goldfish_irq_domain_ops,
> +                                               NULL);
> +     if (!gfpic->irq_domain) {
> +             pr_err("Failed to add irqdomain for Goldfish PIC\n");
> +             ret = -EINVAL;
> +             goto out_iounmap;
> +     }
> +
> +     irq_set_chained_handler_and_data(parent_irq,
> +                                      goldfish_pic_cascade, gfpic);
> +
> +     pr_info("Successfully registered Goldfish PIC\n");
> +     return 0;
> +
> +out_iounmap:
> +     iounmap(gfpic->base);
> +out_unmap_irq:
> +     irq_dispose_mapping(parent_irq);
> +out_free:
> +     kfree(gfpic);
> +out_err:
> +     return ret;
> +}
> +
> +IRQCHIP_DECLARE(google_gf_pic, "google,goldfish-pic", goldfish_pic_of_init);

Thanks,

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

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

* Re: [PATCH v6 5/5] MIPS: ranchu: Add Ranchu as a new generic-based board
  2017-10-30 11:56 ` [PATCH v6 5/5] MIPS: ranchu: Add Ranchu as a new generic-based board Aleksandar Markovic
  2017-10-30 16:45   ` James Hogan
@ 2017-11-01 17:58   ` Paul Burton
  2017-11-02 13:47     ` Miodrag Dinic
  1 sibling, 1 reply; 15+ messages in thread
From: Paul Burton @ 2017-11-01 17:58 UTC (permalink / raw)
  To: Aleksandar Markovic
  Cc: linux-mips, Miodrag Dinic, Goran Ferenc, Aleksandar Markovic,
	David S. Miller, Douglas Leung, Greg Kroah-Hartman, James Hogan,
	linux-kernel, Mauro Carvalho Chehab, Miodrag Dinic, Paul Burton,
	Petar Jovanovic, Raghu Gandham, Ralf Baechle, Randy Dunlap

Hi Aleksandar,

On Mon, Oct 30, 2017 at 12:56:36PM +0100, Aleksandar Markovic wrote:
> diff --git a/arch/mips/generic/board-ranchu.c b/arch/mips/generic/board-ranchu.c
> new file mode 100644
> index 0000000..0397752
> --- /dev/null
> +++ b/arch/mips/generic/board-ranchu.c
> @@ -0,0 +1,79 @@
> +/*
> + * Support code for virtual Ranchu board for MIPS.
> + *
> + * Author: Miodrag Dinic <miodrag.dinic@mips.com>
> + *
> + * 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;  either version 2 of the  License, or (at your
> + * option) any later version.
> + */
> +
> +#include <linux/of_address.h>
> +
> +#include <asm/machine.h>

You should also include asm/mipsregs.h for read_c0_count(), even though it's
presumably being pulled in indirectly as-is.

> +#include <asm/time.h>
> +
> +#define GOLDFISH_TIMER_LOW		0x00
> +#define GOLDFISH_TIMER_HIGH		0x04
> +
> +static __init uint64_t read_rtc_time(void __iomem *base)
> +{
> +	u64 time_low;
> +	u64 time_high;
> +
> +	time_low = readl(base + GOLDFISH_TIMER_LOW);
> +	time_high = readl(base + GOLDFISH_TIMER_HIGH);
> +
> +	return (time_high << 32) | time_low;
> +}
> +
> +static __init unsigned int ranchu_measure_hpt_freq(void)
> +{
> +	u64 rtc_start, rtc_current, rtc_delta;
> +	unsigned int start, count;
> +	struct device_node *np;
> +	void __iomem *rtc_base;
> +
> +	np = of_find_compatible_node(NULL, NULL, "google,goldfish-rtc");
> +	if (!np)
> +		panic("%s(): Failed to find 'google,goldfish-rtc' dt node!",
> +		      __func__);
> +
> +	rtc_base = of_iomap(np, 0);
> +	if (!rtc_base)
> +		panic("%s(): Failed to ioremap Goldfish RTC base!", __func__);
> +
> +	/*
> +	 * poll the nanosecond resolution RTC for 1 second
> +	 * to calibrate the CPU frequency
> +	 */
> +	rtc_start = read_rtc_time(rtc_base);
> +	start = read_c0_count();
> +
> +	do {
> +		rtc_current = read_rtc_time(rtc_base);
> +		rtc_delta = rtc_current - rtc_start;
> +	} while (rtc_delta < NSEC_PER_SEC);
> +
> +	count = read_c0_count() - start;
> +
> +	count += 5000;	/* round */
> +	count -= count % 10000;
> +
> +	return count;
> +}

Would it be possible to have the emulator write the frequency into the
devicetree, as the frequency of a fixed-clock used as the CPU's clock? If that
were possible then there'd be no need for this board setup code at all. Not a
big deal, but it'd be nice.

> +
> +static const struct of_device_id ranchu_of_match[];
> +
> +MIPS_MACHINE(ranchu) = {
> +	.matches = ranchu_of_match,
> +	.measure_hpt_freq = ranchu_measure_hpt_freq,
> +};
> +
> +static const struct of_device_id ranchu_of_match[] = {
> +	{
> +		.compatible = "mti,ranchu",
> +		.data = &__mips_mach_ranchu,
> +	},
> +};

Could you move ranchu_of_match before the MIPS_MACHINE & drop the forward
declaration? That would feel tidier to me. It could also be marked as
__initdata.

In general though, with those & James' comments addressed, I think this is
looking good.

Thanks,
    Paul

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

* RE: [PATCH v6 5/5] MIPS: ranchu: Add Ranchu as a new generic-based board
  2017-10-30 16:45   ` James Hogan
@ 2017-11-02 12:47     ` Miodrag Dinic
  2017-11-02 12:53       ` James Hogan
  0 siblings, 1 reply; 15+ messages in thread
From: Miodrag Dinic @ 2017-11-02 12:47 UTC (permalink / raw)
  To: James Hogan, Aleksandar Markovic
  Cc: linux-mips, Goran Ferenc, Aleksandar Markovic, David S. Miller,
	Douglas Leung, Greg Kroah-Hartman, linux-kernel,
	Mauro Carvalho Chehab, Miodrag Dinic, Paul Burton, Paul Burton,
	Petar Jovanovic, Raghu Gandham, Ralf Baechle, Randy Dunlap

Hello James,

sorry for a late reply, please find the answers in-lined :

> On Mon, Oct 30, 2017 at 12:56:36PM +0100, Aleksandar Markovic wrote:
> > From: Miodrag Dinic <miodrag.dinic@mips.com>
> > 
> > Provide amendments to the MIPS generic platform framework so that
> > the new generic-based board Ranchu can be chosen to be built.
> 
> A bit more info about the board would be good here. What boot protocol
> is used? Does QEMU generate the DT dynamically?

I agree, we will fill the commit message with necessary information about
the newly introduced virtual board.

> > 
> > Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
> > Signed-off-by: Goran Ferenc <goran.ferenc@mips.com>
> > Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
> > ---
> >  MAINTAINERS                                   |  6 ++
> >  arch/mips/configs/generic/board-ranchu.config | 30 ++++++++++
> >  arch/mips/generic/Kconfig                     | 10 ++++
> >  arch/mips/generic/Makefile                    |  1 +
> >  arch/mips/generic/board-ranchu.c              | 79 +++++++++++++++++++++++++++
> >  5 files changed, 126 insertions(+)
> >  create mode 100644 arch/mips/configs/generic/board-ranchu.config
> >  create mode 100644 arch/mips/generic/board-ranchu.c
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index f1be016..e429cc2 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -11308,6 +11308,12 @@ S:   Maintained
> >  F:   Documentation/blockdev/ramdisk.txt
> >  F:   drivers/block/brd.c
> >  
> > +RANCHU VIRTUAL BOARD FOR MIPS
> > +M:   Miodrag Dinic <miodrag.dinic@mips.com>
> > +L:   linux-mips@linux-mips.org
> > +S:   Supported
> > +F:   arch/mips/generic/board-ranchu.c
> 
> Maybe worth adding arch/mips/configs/generic/board-ranchu.config too.

It will be addressed in V7.

> > +
> >  RANDOM NUMBER DRIVER
> >  M:   "Theodore Ts'o" <tytso@mit.edu>
> >  S:   Maintained
> 
> > diff --git a/arch/mips/generic/Kconfig b/arch/mips/generic/Kconfig
> > index e0436aa..93582be 100644
> > --- a/arch/mips/generic/Kconfig
> > +++ b/arch/mips/generic/Kconfig
> > @@ -42,4 +42,14 @@ config FIT_IMAGE_FDT_NI169445
> >          Enable this to include the FDT for the 169445 platform from
> >          National Instruments in the FIT kernel image.
> >  
> > +config VIRT_BOARD_RANCHU
> > +     bool "Ranchu platform for Android emulator"
> > +     help
> > +       This enables support for the platform used by Android emulator.
> > +
> > +       Ranchu platform consists of a set of virtual devices. This platform
> > +       enables emulation of variety of virtual configurations while using
> > +       Android emulator. Android emulator is based on Qemu, and contains
> > +       the support for the same set of virtual devices.
> 
> This is effectively in the section "FIT/UHI Boards", but it has a
> platform file and no DT/FIT stuff in tree a bit like the boards in the
> section "Legacy (non-UHI/non-FIT) Boards".
> 
> I'm guessing it might be something in between, with UHI + platform code,
> but DT provided by QEMU (i.e. FIT support makes no sense)?

The Ranchu emulator was designed to provide the DT dynamically generated on
the QEMU side and passed to the kernel using UHI boot protocol in DTB handover mode.
Currently we do not support FIT, but I think it is doable to enable it
at some point later in time when we stop supporting android 3.10/3.18 kernels
in Ranchu.

> If it uses UHI I suppose it doesn't belong in the legacy section, but I
> think a consistent prompt would be beneficial, e.g.
> 
> +config VIRT_BOARD_RANCHU
> +       bool "Support Ranchu platform for Android emulator"
> ...

It will be addressed in V7.

> > diff --git a/arch/mips/generic/board-ranchu.c b/arch/mips/generic/board-ranchu.c
> > new file mode 100644
> > index 0000000..0397752
> > --- /dev/null
> > +++ b/arch/mips/generic/board-ranchu.c
> > @@ -0,0 +1,79 @@
> > +/*
> > + * Support code for virtual Ranchu board for MIPS.
> > + *
> > + * Author: Miodrag Dinic <miodrag.dinic@mips.com>
> > + *
> > + * 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;  either version 2 of the  License, or (at your
> > + * option) any later version.
> > + */
> > +
> > +#include <linux/of_address.h>
> > +
> > +#include <asm/machine.h>
> > +#include <asm/time.h>
> > +
> > +#define GOLDFISH_TIMER_LOW           0x00
> > +#define GOLDFISH_TIMER_HIGH          0x04
> > +
> > +static __init uint64_t read_rtc_time(void __iomem *base)
> > +{
> > +     u64 time_low;
> > +     u64 time_high;
> > +
> > +     time_low = readl(base + GOLDFISH_TIMER_LOW);
> > +     time_high = readl(base + GOLDFISH_TIMER_HIGH);
> > +
> > +     return (time_high << 32) | time_low;
> 
> What if high changes while reading this?
> 
> E.g.
> TIMER_LOW        0x00000000 *0xffffffff*
> TIMER_HIGH      *0x00000001* 0x00000000
> 
> You'd presumably get 0x00000001ffffffff.
> 
> Perhaps it should read HIGH before too, and retry if it has changed.

This was already discussed in some earlier posts. (https://patchwork.linux-mips.org/patch/16628/)
Reading the low value first actually latches the high value,
so it is safe to leave it this way. Here is the relevant RTC device
implementation in QEMU:

static uint64_t goldfish_rtc_read(void *opaque, hwaddr offset, unsigned size)
{
    struct rtc_state *s = (struct rtc_state *)opaque;
    switch(offset) {
        case TIMER_TIME_LOW:
            s->now_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + s->time_base;
            return s->now_ns;
        case TIMER_TIME_HIGH:
            return s->now_ns >> 32;

Kind regards,
Miodrag


________________________________________
From: James Hogan
Sent: Monday, October 30, 2017 5:45 PM
To: Aleksandar Markovic
Cc: linux-mips@linux-mips.org; Miodrag Dinic; Goran Ferenc; Aleksandar Markovic; David S. Miller; Douglas Leung; Greg Kroah-Hartman; linux-kernel@vger.kernel.org; Mauro Carvalho Chehab; Miodrag Dinic; Paul Burton; Paul Burton; Petar Jovanovic; Raghu Gandham; Ralf Baechle; Randy Dunlap
Subject: Re: [PATCH v6 5/5] MIPS: ranchu: Add Ranchu as a new generic-based board

On Mon, Oct 30, 2017 at 12:56:36PM +0100, Aleksandar Markovic wrote:
> From: Miodrag Dinic <miodrag.dinic@mips.com>
>
> Provide amendments to the MIPS generic platform framework so that
> the new generic-based board Ranchu can be chosen to be built.

A bit more info about the board would be good here. What boot protocol
is used? Does QEMU generate the DT dynamically?

>
> Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
> Signed-off-by: Goran Ferenc <goran.ferenc@mips.com>
> Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
> ---
>  MAINTAINERS                                   |  6 ++
>  arch/mips/configs/generic/board-ranchu.config | 30 ++++++++++
>  arch/mips/generic/Kconfig                     | 10 ++++
>  arch/mips/generic/Makefile                    |  1 +
>  arch/mips/generic/board-ranchu.c              | 79 +++++++++++++++++++++++++++
>  5 files changed, 126 insertions(+)
>  create mode 100644 arch/mips/configs/generic/board-ranchu.config
>  create mode 100644 arch/mips/generic/board-ranchu.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f1be016..e429cc2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11308,6 +11308,12 @@ S:   Maintained
>  F:   Documentation/blockdev/ramdisk.txt
>  F:   drivers/block/brd.c
>
> +RANCHU VIRTUAL BOARD FOR MIPS
> +M:   Miodrag Dinic <miodrag.dinic@mips.com>
> +L:   linux-mips@linux-mips.org
> +S:   Supported
> +F:   arch/mips/generic/board-ranchu.c

Maybe worth adding arch/mips/configs/generic/board-ranchu.config too.

> +
>  RANDOM NUMBER DRIVER
>  M:   "Theodore Ts'o" <tytso@mit.edu>
>  S:   Maintained

> diff --git a/arch/mips/generic/Kconfig b/arch/mips/generic/Kconfig
> index e0436aa..93582be 100644
> --- a/arch/mips/generic/Kconfig
> +++ b/arch/mips/generic/Kconfig
> @@ -42,4 +42,14 @@ config FIT_IMAGE_FDT_NI169445
>         Enable this to include the FDT for the 169445 platform from
>         National Instruments in the FIT kernel image.
>
> +config VIRT_BOARD_RANCHU
> +     bool "Ranchu platform for Android emulator"
> +     help
> +       This enables support for the platform used by Android emulator.
> +
> +       Ranchu platform consists of a set of virtual devices. This platform
> +       enables emulation of variety of virtual configurations while using
> +       Android emulator. Android emulator is based on Qemu, and contains
> +       the support for the same set of virtual devices.

This is effectively in the section "FIT/UHI Boards", but it has a
platform file and no DT/FIT stuff in tree a bit like the boards in the
section "Legacy (non-UHI/non-FIT) Boards".

I'm guessing it might be something in between, with UHI + platform code,
but DT provided by QEMU (i.e. FIT support makes no sense)?

If it uses UHI I suppose it doesn't belong in the legacy section, but I
think a consistent prompt would be beneficial, e.g.

+config VIRT_BOARD_RANCHU
+       bool "Support Ranchu platform for Android emulator"
..

> diff --git a/arch/mips/generic/board-ranchu.c b/arch/mips/generic/board-ranchu.c
> new file mode 100644
> index 0000000..0397752
> --- /dev/null
> +++ b/arch/mips/generic/board-ranchu.c
> @@ -0,0 +1,79 @@
> +/*
> + * Support code for virtual Ranchu board for MIPS.
> + *
> + * Author: Miodrag Dinic <miodrag.dinic@mips.com>
> + *
> + * 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;  either version 2 of the  License, or (at your
> + * option) any later version.
> + */
> +
> +#include <linux/of_address.h>
> +
> +#include <asm/machine.h>
> +#include <asm/time.h>
> +
> +#define GOLDFISH_TIMER_LOW           0x00
> +#define GOLDFISH_TIMER_HIGH          0x04
> +
> +static __init uint64_t read_rtc_time(void __iomem *base)
> +{
> +     u64 time_low;
> +     u64 time_high;
> +
> +     time_low = readl(base + GOLDFISH_TIMER_LOW);
> +     time_high = readl(base + GOLDFISH_TIMER_HIGH);
> +
> +     return (time_high << 32) | time_low;

What if high changes while reading this?

E.g.
TIMER_LOW        0x00000000 *0xffffffff*
TIMER_HIGH      *0x00000001* 0x00000000

You'd presumably get 0x00000001ffffffff.

Perhaps it should read HIGH before too, and retry if it has changed.

Cheers
James

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

* Re: [PATCH v6 5/5] MIPS: ranchu: Add Ranchu as a new generic-based board
  2017-11-02 12:47     ` Miodrag Dinic
@ 2017-11-02 12:53       ` James Hogan
  0 siblings, 0 replies; 15+ messages in thread
From: James Hogan @ 2017-11-02 12:53 UTC (permalink / raw)
  To: Miodrag Dinic
  Cc: Aleksandar Markovic, linux-mips, Goran Ferenc,
	Aleksandar Markovic, David S. Miller, Douglas Leung,
	Greg Kroah-Hartman, linux-kernel, Mauro Carvalho Chehab,
	Miodrag Dinic, Paul Burton, Paul Burton, Petar Jovanovic,
	Raghu Gandham, Ralf Baechle, Randy Dunlap

On Thu, Nov 02, 2017 at 12:47:27PM +0000, Miodrag Dinic wrote:
> > > +static __init uint64_t read_rtc_time(void __iomem *base)
> > > +{
> > > +     u64 time_low;
> > > +     u64 time_high;
> > > +
> > > +     time_low = readl(base + GOLDFISH_TIMER_LOW);
> > > +     time_high = readl(base + GOLDFISH_TIMER_HIGH);
> > > +
> > > +     return (time_high << 32) | time_low;
> > 
> > What if high changes while reading this?
> > 
> > E.g.
> > TIMER_LOW        0x00000000 *0xffffffff*
> > TIMER_HIGH      *0x00000001* 0x00000000
> > 
> > You'd presumably get 0x00000001ffffffff.
> > 
> > Perhaps it should read HIGH before too, and retry if it has changed.
> 
> This was already discussed in some earlier posts. (https://patchwork.linux-mips.org/patch/16628/)
> Reading the low value first actually latches the high value,
> so it is safe to leave it this way. Here is the relevant RTC device
> implementation in QEMU:
> 
> static uint64_t goldfish_rtc_read(void *opaque, hwaddr offset, unsigned size)
> {
>     struct rtc_state *s = (struct rtc_state *)opaque;
>     switch(offset) {
>         case TIMER_TIME_LOW:
>             s->now_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + s->time_base;
>             return s->now_ns;
>         case TIMER_TIME_HIGH:
>             return s->now_ns >> 32;

Ah okay. In that case the side effect of reading low clearly isn't
obvious enough reading the driver code and it needs a comment to clarify
the hardware behaviour :-)

Cheers
James

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

* RE: [PATCH v6 5/5] MIPS: ranchu: Add Ranchu as a new generic-based board
  2017-11-01 17:58   ` Paul Burton
@ 2017-11-02 13:47     ` Miodrag Dinic
  2017-11-02 20:49       ` Paul Burton
  0 siblings, 1 reply; 15+ messages in thread
From: Miodrag Dinic @ 2017-11-02 13:47 UTC (permalink / raw)
  To: Paul Burton, Aleksandar Markovic
  Cc: linux-mips, Goran Ferenc, Aleksandar Markovic, David S. Miller,
	Douglas Leung, Greg Kroah-Hartman, James Hogan, linux-kernel,
	Mauro Carvalho Chehab, Miodrag Dinic, Paul Burton,
	Petar Jovanovic, Raghu Gandham, Ralf Baechle, Randy Dunlap

Hi Paul,

thank you for the review. Please find the answers in-lined:

> > +#include <linux/of_address.h>
> > +
> > +#include <asm/machine.h>
> 
> You should also include asm/mipsregs.h for read_c0_count(), even though it's
> presumably being pulled in indirectly as-is.

Thanks, we will address this in V7.

> > +#include <asm/time.h>
> > +
> > +#define GOLDFISH_TIMER_LOW           0x00
> > +#define GOLDFISH_TIMER_HIGH          0x04
> > +
> > +static __init uint64_t read_rtc_time(void __iomem *base)
> > +{
> > +     u64 time_low;
> > +     u64 time_high;
> > +
> > +     time_low = readl(base + GOLDFISH_TIMER_LOW);
> > +     time_high = readl(base + GOLDFISH_TIMER_HIGH);
> > +
> > +     return (time_high << 32) | time_low;
> > +}
> > +
> > +static __init unsigned int ranchu_measure_hpt_freq(void)
> > +{
> > +     u64 rtc_start, rtc_current, rtc_delta;
> > +     unsigned int start, count;
> > +     struct device_node *np;
> > +     void __iomem *rtc_base;
> > +
> > +     np = of_find_compatible_node(NULL, NULL, "google,goldfish-rtc");
> > +     if (!np)
> > +             panic("%s(): Failed to find 'google,goldfish-rtc' dt node!",
> > +                   __func__);
> > +
> > +     rtc_base = of_iomap(np, 0);
> > +     if (!rtc_base)
> > +             panic("%s(): Failed to ioremap Goldfish RTC base!", __func__);
> > +
> > +     /*
> > +      * poll the nanosecond resolution RTC for 1 second
> > +      * to calibrate the CPU frequency
> > +      */
> > +     rtc_start = read_rtc_time(rtc_base);
> > +     start = read_c0_count();
> > +
> > +     do {
> > +             rtc_current = read_rtc_time(rtc_base);
> > +             rtc_delta = rtc_current - rtc_start;
> > +     } while (rtc_delta < NSEC_PER_SEC);
> > +
> > +     count = read_c0_count() - start;
> > +
> > +     count += 5000;  /* round */
> > +     count -= count % 10000;
> > +
> > +     return count;
> > +}
> 
> Would it be possible to have the emulator write the frequency into the
> devicetree, as the frequency of a fixed-clock used as the CPU's clock? If that
> were possible then there'd be no need for this board setup code at all. Not a
> big deal, but it'd be nice.

Well, yes I think that would be possible, but since this will be run on the emulator,
fixed-clock may not be the best because the point of this code is to make emulator
calibrate itself according to speed of the host which runs the emulation.
We may consider more advanced approach on this issue in the future, but for now
this works fine for us.

> > +
> > +static const struct of_device_id ranchu_of_match[];
> > +
> > +MIPS_MACHINE(ranchu) = {
> > +     .matches = ranchu_of_match,
> > +     .measure_hpt_freq = ranchu_measure_hpt_freq,
> > +};
> > +
> > +static const struct of_device_id ranchu_of_match[] = {
> > +     {
> > +             .compatible = "mti,ranchu",
> > +             .data = &__mips_mach_ranchu,
> > +     },
> > +};
> 
> Could you move ranchu_of_match before the MIPS_MACHINE & drop the forward
> declaration? That would feel tidier to me. It could also be marked as
> __initdata.

We can not remove the forward declaration because we need to define
__mips_mach_ranchu (which is done by MIPS_MACHINE(ranchu)) before ranchu_of_match.

Kind regards,
Miodrag

________________________________________
From: Paul Burton [paul.burton@mips.com]
Sent: Wednesday, November 1, 2017 6:58 PM
To: Aleksandar Markovic
Cc: linux-mips@linux-mips.org; Miodrag Dinic; Goran Ferenc; Aleksandar Markovic; David S. Miller; Douglas Leung; Greg Kroah-Hartman; James Hogan; linux-kernel@vger.kernel.org; Mauro Carvalho Chehab; Miodrag Dinic; Paul Burton; Petar Jovanovic; Raghu Gandham; Ralf Baechle; Randy Dunlap
Subject: Re: [PATCH v6 5/5] MIPS: ranchu: Add Ranchu as a new generic-based board

Hi Aleksandar,

On Mon, Oct 30, 2017 at 12:56:36PM +0100, Aleksandar Markovic wrote:
> diff --git a/arch/mips/generic/board-ranchu.c b/arch/mips/generic/board-ranchu.c
> new file mode 100644
> index 0000000..0397752
> --- /dev/null
> +++ b/arch/mips/generic/board-ranchu.c
> @@ -0,0 +1,79 @@
> +/*
> + * Support code for virtual Ranchu board for MIPS.
> + *
> + * Author: Miodrag Dinic <miodrag.dinic@mips.com>
> + *
> + * 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;  either version 2 of the  License, or (at your
> + * option) any later version.
> + */
> +
> +#include <linux/of_address.h>
> +
> +#include <asm/machine.h>

You should also include asm/mipsregs.h for read_c0_count(), even though it's
presumably being pulled in indirectly as-is.

> +#include <asm/time.h>
> +
> +#define GOLDFISH_TIMER_LOW           0x00
> +#define GOLDFISH_TIMER_HIGH          0x04
> +
> +static __init uint64_t read_rtc_time(void __iomem *base)
> +{
> +     u64 time_low;
> +     u64 time_high;
> +
> +     time_low = readl(base + GOLDFISH_TIMER_LOW);
> +     time_high = readl(base + GOLDFISH_TIMER_HIGH);
> +
> +     return (time_high << 32) | time_low;
> +}
> +
> +static __init unsigned int ranchu_measure_hpt_freq(void)
> +{
> +     u64 rtc_start, rtc_current, rtc_delta;
> +     unsigned int start, count;
> +     struct device_node *np;
> +     void __iomem *rtc_base;
> +
> +     np = of_find_compatible_node(NULL, NULL, "google,goldfish-rtc");
> +     if (!np)
> +             panic("%s(): Failed to find 'google,goldfish-rtc' dt node!",
> +                   __func__);
> +
> +     rtc_base = of_iomap(np, 0);
> +     if (!rtc_base)
> +             panic("%s(): Failed to ioremap Goldfish RTC base!", __func__);
> +
> +     /*
> +      * poll the nanosecond resolution RTC for 1 second
> +      * to calibrate the CPU frequency
> +      */
> +     rtc_start = read_rtc_time(rtc_base);
> +     start = read_c0_count();
> +
> +     do {
> +             rtc_current = read_rtc_time(rtc_base);
> +             rtc_delta = rtc_current - rtc_start;
> +     } while (rtc_delta < NSEC_PER_SEC);
> +
> +     count = read_c0_count() - start;
> +
> +     count += 5000;  /* round */
> +     count -= count % 10000;
> +
> +     return count;
> +}

Would it be possible to have the emulator write the frequency into the
devicetree, as the frequency of a fixed-clock used as the CPU's clock? If that
were possible then there'd be no need for this board setup code at all. Not a
big deal, but it'd be nice.

> +
> +static const struct of_device_id ranchu_of_match[];
> +
> +MIPS_MACHINE(ranchu) = {
> +     .matches = ranchu_of_match,
> +     .measure_hpt_freq = ranchu_measure_hpt_freq,
> +};
> +
> +static const struct of_device_id ranchu_of_match[] = {
> +     {
> +             .compatible = "mti,ranchu",
> +             .data = &__mips_mach_ranchu,
> +     },
> +};

Could you move ranchu_of_match before the MIPS_MACHINE & drop the forward
declaration? That would feel tidier to me. It could also be marked as
__initdata.

In general though, with those & James' comments addressed, I think this is
looking good.

Thanks,
    Paul

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

* Re: [PATCH v6 5/5] MIPS: ranchu: Add Ranchu as a new generic-based board
  2017-11-02 13:47     ` Miodrag Dinic
@ 2017-11-02 20:49       ` Paul Burton
  2017-11-03 14:04         ` Miodrag Dinic
  0 siblings, 1 reply; 15+ messages in thread
From: Paul Burton @ 2017-11-02 20:49 UTC (permalink / raw)
  To: Miodrag Dinic
  Cc: Aleksandar Markovic, linux-mips, Goran Ferenc,
	Aleksandar Markovic, David S. Miller, Douglas Leung,
	Greg Kroah-Hartman, James Hogan, linux-kernel,
	Mauro Carvalho Chehab, Miodrag Dinic, Paul Burton,
	Petar Jovanovic, Raghu Gandham, Ralf Baechle, Randy Dunlap

Hi Miodrag,

On Thu, Nov 02, 2017 at 06:47:05AM -0700, Miodrag Dinic wrote:
> > > +static const struct of_device_id ranchu_of_match[];
> > > +
> > > +MIPS_MACHINE(ranchu) = {
> > > +     .matches = ranchu_of_match,
> > > +     .measure_hpt_freq = ranchu_measure_hpt_freq,
> > > +};
> > > +
> > > +static const struct of_device_id ranchu_of_match[] = {
> > > +     {
> > > +             .compatible = "mti,ranchu",
> > > +             .data = &__mips_mach_ranchu,
> > > +     },
> > > +};
> > 
> > Could you move ranchu_of_match before the MIPS_MACHINE & drop the forward
> > declaration? That would feel tidier to me. It could also be marked as
> > __initdata.
> 
> We can not remove the forward declaration because we need to define
> __mips_mach_ranchu (which is done by MIPS_MACHINE(ranchu)) before ranchu_of_match.

Why? Why do you need to set the struct of_device_id data field? The generic
code only provides the match data to the machine fixup_fdt callback which you
don't implement, so the value is never used.

Thanks,
    Paul

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

* RE: [PATCH v6 5/5] MIPS: ranchu: Add Ranchu as a new generic-based board
  2017-11-02 20:49       ` Paul Burton
@ 2017-11-03 14:04         ` Miodrag Dinic
  0 siblings, 0 replies; 15+ messages in thread
From: Miodrag Dinic @ 2017-11-03 14:04 UTC (permalink / raw)
  To: Paul Burton
  Cc: Aleksandar Markovic, linux-mips, Goran Ferenc,
	Aleksandar Markovic, David S. Miller, Douglas Leung,
	Greg Kroah-Hartman, James Hogan, linux-kernel,
	Mauro Carvalho Chehab, Miodrag Dinic, Paul Burton,
	Petar Jovanovic, Raghu Gandham, Ralf Baechle, Randy Dunlap

Hi Paul,

> > We can not remove the forward declaration because we need to define
> > __mips_mach_ranchu (which is done by MIPS_MACHINE(ranchu)) before ranchu_of_match.
> 
> Why? Why do you need to set the struct of_device_id data field? The generic
> code only provides the match data to the machine fixup_fdt callback which you
> don't implement, so the value is never used.

Oooh, yes you are right. This was a leftover when we started porting Ranchu as a legacy platform.
We will fix it in V8. Thank you.

Kind regards,
Miodrag

________________________________________
From: Paul Burton [paul.burton@mips.com]
Sent: Thursday, November 2, 2017 9:49 PM
To: Miodrag Dinic
Cc: Aleksandar Markovic; linux-mips@linux-mips.org; Goran Ferenc; Aleksandar Markovic; David S. Miller; Douglas Leung; Greg Kroah-Hartman; James Hogan; linux-kernel@vger.kernel.org; Mauro Carvalho Chehab; Miodrag Dinic; Paul Burton; Petar Jovanovic; Raghu Gandham; Ralf Baechle; Randy Dunlap
Subject: Re: [PATCH v6 5/5] MIPS: ranchu: Add Ranchu as a new generic-based board

Hi Miodrag,

On Thu, Nov 02, 2017 at 06:47:05AM -0700, Miodrag Dinic wrote:
> > > +static const struct of_device_id ranchu_of_match[];
> > > +
> > > +MIPS_MACHINE(ranchu) = {
> > > +     .matches = ranchu_of_match,
> > > +     .measure_hpt_freq = ranchu_measure_hpt_freq,
> > > +};
> > > +
> > > +static const struct of_device_id ranchu_of_match[] = {
> > > +     {
> > > +             .compatible = "mti,ranchu",
> > > +             .data = &__mips_mach_ranchu,
> > > +     },
> > > +};
> >
> > Could you move ranchu_of_match before the MIPS_MACHINE & drop the forward
> > declaration? That would feel tidier to me. It could also be marked as
> > __initdata.
>
> We can not remove the forward declaration because we need to define
> __mips_mach_ranchu (which is done by MIPS_MACHINE(ranchu)) before ranchu_of_match.

Why? Why do you need to set the struct of_device_id data field? The generic
code only provides the match data to the machine fixup_fdt callback which you
don't implement, so the value is never used.

Thanks,
    Paul

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

end of thread, other threads:[~2017-11-03 14:10 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-30 11:56 [PATCH v6 0/5] MIPS: Add virtual Ranchu board as a generic-based board Aleksandar Markovic
2017-10-30 11:56 ` [PATCH v6 1/5] Documentation: Add device tree binding for Goldfish PIC driver Aleksandar Markovic
2017-10-30 11:56 ` [PATCH v6 2/5] irqchip/irq-goldfish-pic: Add " Aleksandar Markovic
2017-10-31  2:26   ` Marc Zyngier
2017-11-01 14:34     ` Miodrag Dinic
2017-10-30 11:56 ` [PATCH v6 3/5] Documentation: Add device tree binding for Goldfish FB driver Aleksandar Markovic
2017-10-30 11:56 ` [PATCH v6 4/5] video: goldfishfb: Add support for device tree bindings Aleksandar Markovic
2017-10-30 11:56 ` [PATCH v6 5/5] MIPS: ranchu: Add Ranchu as a new generic-based board Aleksandar Markovic
2017-10-30 16:45   ` James Hogan
2017-11-02 12:47     ` Miodrag Dinic
2017-11-02 12:53       ` James Hogan
2017-11-01 17:58   ` Paul Burton
2017-11-02 13:47     ` Miodrag Dinic
2017-11-02 20:49       ` Paul Burton
2017-11-03 14:04         ` Miodrag Dinic

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).