All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alban <albeu@free.fr>
To: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
Cc: Alban <albeu@free.fr>,
	ralf@linux-mips.org, robh+dt@kernel.org,
	linus.walleij@linaro.org, linux-mips@linux-mips.org,
	linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/5] MIPS: xilfpga: Add mipsfpga platform code
Date: Wed, 21 Oct 2015 16:35:02 +0200	[thread overview]
Message-ID: <20151021163502.44be785f@avionic-0020> (raw)
In-Reply-To: <1444827117-10939-5-git-send-email-Zubair.Kakakhel@imgtec.com>

Am Wed, 14 Oct 2015 13:51:56 +0100
schrieb Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>:

> The xilfpga platform will be DT only.
> 
> Add required platform code. DT files have already been added separately
>
> [...]
>
> diff --git a/arch/mips/include/asm/mach-xilfpga/gpio.h b/arch/mips/include/asm/mach-xilfpga/gpio.h
> new file mode 100644
> index 0000000..26778fc
> --- /dev/null
> +++ b/arch/mips/include/asm/mach-xilfpga/gpio.h
> @@ -0,0 +1,19 @@
> +/*
> + * Copyright (C) 2015 Imagination Technologies
> + * Author: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.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.
> + */
> +
> +#ifndef __ASM_MACH_XILFPGA_GPIO_H
> +#define __ASM_MACH_XILFPGA_GPIO_H
> +
> +#include <asm-generic/gpio.h>
> +
> +#define gpio_get_value __gpio_get_value
> +#define gpio_set_value __gpio_set_value
> +
> +#endif /* __ASM_MACH_XILFPGA_GPIO_H */

Custom gpio.h has been disabled, this file won't be used, it can just
be dropped.

> diff --git a/arch/mips/include/asm/mach-xilfpga/irq.h b/arch/mips/include/asm/mach-xilfpga/irq.h
> new file mode 100644
> index 0000000..0132a5b9
> --- /dev/null
> +++ b/arch/mips/include/asm/mach-xilfpga/irq.h
> @@ -0,0 +1,18 @@
> +/*
> + * Copyright (C) 2015 Imagination Technologies
> + * Author: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.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.
> + */
> +
> +#ifndef __MIPS_ASM_MACH_XILFPGA_IRQ_H__
> +#define __MIPS_ASM_MACH_XILFPGA_IRQ_H__
> +
> +#define NR_IRQS 32
> +
> +#include_next <irq.h>
> +
> +#endif /* __MIPS_ASM_MACH_XILFPGA_IRQ_H__ */

Is this really needed? If not I would not add it to keep the maintenance
burden down.

> [...]
>
> diff --git a/arch/mips/xilfpga/time.c b/arch/mips/xilfpga/time.c
> new file mode 100644
> index 0000000..3f2e39e
> --- /dev/null
> +++ b/arch/mips/xilfpga/time.c
> @@ -0,0 +1,41 @@
> +/*
> + * Xilfpga clocksource/timer setup
> + *
> + * Copyright (C) 2015 Imagination Technologies
> + * Author: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/clocksource.h>
> +#include <linux/of.h>
> +
> +#include <asm/time.h>
> +
> +void __init plat_time_init(void)
> +{
> +	struct device_node *np;
> +	struct clk *clk = 0;
> +
> +	of_clk_init(NULL);
> +	clocksource_of_init();
> +
> +	np = of_get_cpu_node(0, NULL);
> +	if (!np) {
> +		pr_err("Failed to get CPU node\n");
> +		return;
> +	}
> +
> +	clk = of_clk_get(np, 0);
> +	if (IS_ERR(clk)) {
> +		pr_err("Failed to get CPU clock: %ld\n", PTR_ERR(clk));
> +		return;
> +	}
> +
> +	mips_hpt_frequency = clk_get_rate(clk) / 2;
> +	clk_put(clk);
> +}

I wanted to use something similar on ATH79 once all boards moved to DT.
On the other hand I saw that BMIPS use a dedicated property on the CPU
node to represent the HPT frequency. If possible it would make sense to
use a common scheme for all MIPS platforms. However I personally don't
know enough about MIPS in general to judge this.

On the other hand I think it might make more sense to have a dedicated
node in DT to represent the HPT timer instead of hacking on the CPU
node. That would also fit better if the HPT code ever become is a real
driver.

Alban

WARNING: multiple messages have this Message-ID (diff)
From: Alban <albeu@free.fr>
To: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
Cc: Alban <albeu@free.fr>, <ralf@linux-mips.org>,
	<robh+dt@kernel.org>, <linus.walleij@linaro.org>,
	<linux-mips@linux-mips.org>, <linux-gpio@vger.kernel.org>,
	<devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 4/5] MIPS: xilfpga: Add mipsfpga platform code
Date: Wed, 21 Oct 2015 16:35:02 +0200	[thread overview]
Message-ID: <20151021163502.44be785f@avionic-0020> (raw)
In-Reply-To: <1444827117-10939-5-git-send-email-Zubair.Kakakhel@imgtec.com>

Am Wed, 14 Oct 2015 13:51:56 +0100
schrieb Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>:

> The xilfpga platform will be DT only.
> 
> Add required platform code. DT files have already been added separately
>
> [...]
>
> diff --git a/arch/mips/include/asm/mach-xilfpga/gpio.h b/arch/mips/include/asm/mach-xilfpga/gpio.h
> new file mode 100644
> index 0000000..26778fc
> --- /dev/null
> +++ b/arch/mips/include/asm/mach-xilfpga/gpio.h
> @@ -0,0 +1,19 @@
> +/*
> + * Copyright (C) 2015 Imagination Technologies
> + * Author: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.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.
> + */
> +
> +#ifndef __ASM_MACH_XILFPGA_GPIO_H
> +#define __ASM_MACH_XILFPGA_GPIO_H
> +
> +#include <asm-generic/gpio.h>
> +
> +#define gpio_get_value __gpio_get_value
> +#define gpio_set_value __gpio_set_value
> +
> +#endif /* __ASM_MACH_XILFPGA_GPIO_H */

Custom gpio.h has been disabled, this file won't be used, it can just
be dropped.

> diff --git a/arch/mips/include/asm/mach-xilfpga/irq.h b/arch/mips/include/asm/mach-xilfpga/irq.h
> new file mode 100644
> index 0000000..0132a5b9
> --- /dev/null
> +++ b/arch/mips/include/asm/mach-xilfpga/irq.h
> @@ -0,0 +1,18 @@
> +/*
> + * Copyright (C) 2015 Imagination Technologies
> + * Author: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.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.
> + */
> +
> +#ifndef __MIPS_ASM_MACH_XILFPGA_IRQ_H__
> +#define __MIPS_ASM_MACH_XILFPGA_IRQ_H__
> +
> +#define NR_IRQS 32
> +
> +#include_next <irq.h>
> +
> +#endif /* __MIPS_ASM_MACH_XILFPGA_IRQ_H__ */

Is this really needed? If not I would not add it to keep the maintenance
burden down.

> [...]
>
> diff --git a/arch/mips/xilfpga/time.c b/arch/mips/xilfpga/time.c
> new file mode 100644
> index 0000000..3f2e39e
> --- /dev/null
> +++ b/arch/mips/xilfpga/time.c
> @@ -0,0 +1,41 @@
> +/*
> + * Xilfpga clocksource/timer setup
> + *
> + * Copyright (C) 2015 Imagination Technologies
> + * Author: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/clocksource.h>
> +#include <linux/of.h>
> +
> +#include <asm/time.h>
> +
> +void __init plat_time_init(void)
> +{
> +	struct device_node *np;
> +	struct clk *clk = 0;
> +
> +	of_clk_init(NULL);
> +	clocksource_of_init();
> +
> +	np = of_get_cpu_node(0, NULL);
> +	if (!np) {
> +		pr_err("Failed to get CPU node\n");
> +		return;
> +	}
> +
> +	clk = of_clk_get(np, 0);
> +	if (IS_ERR(clk)) {
> +		pr_err("Failed to get CPU clock: %ld\n", PTR_ERR(clk));
> +		return;
> +	}
> +
> +	mips_hpt_frequency = clk_get_rate(clk) / 2;
> +	clk_put(clk);
> +}

I wanted to use something similar on ATH79 once all boards moved to DT.
On the other hand I saw that BMIPS use a dedicated property on the CPU
node to represent the HPT frequency. If possible it would make sense to
use a common scheme for all MIPS platforms. However I personally don't
know enough about MIPS in general to judge this.

On the other hand I think it might make more sense to have a dedicated
node in DT to represent the HPT timer instead of hacking on the CPU
node. That would also fit better if the HPT code ever become is a real
driver.

Alban

  parent reply	other threads:[~2015-10-21 14:35 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-14 12:51 [PATCH 0/5] MIPS: Add Xilfpga platform Zubair Lutfullah Kakakhel
2015-10-14 12:51 ` Zubair Lutfullah Kakakhel
     [not found] ` <1444827117-10939-1-git-send-email-Zubair.Kakakhel-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
2015-10-14 12:51   ` [PATCH 1/5] dt-bindings: MIPS: Document xilfpga bindings and boot style Zubair Lutfullah Kakakhel
2015-10-14 12:51     ` Zubair Lutfullah Kakakhel
2015-10-14 12:51     ` Zubair Lutfullah Kakakhel
2015-10-15  7:22     ` James Hogan
2015-10-15  7:22       ` James Hogan
2015-10-18 16:47     ` Moritz Fischer
2015-10-14 12:51 ` [PATCH 2/5] gpio/xilinx: enable for MIPS Zubair Lutfullah Kakakhel
2015-10-14 12:51   ` Zubair Lutfullah Kakakhel
2015-10-14 15:18   ` Sören Brinkmann
2015-10-14 15:18     ` Sören Brinkmann
2015-10-14 15:57     ` Lars-Peter Clausen
2015-10-14 16:54       ` Sören Brinkmann
2015-10-14 16:54         ` Sören Brinkmann
2015-10-14 17:24         ` Lars-Peter Clausen
2015-10-14 17:24           ` Lars-Peter Clausen
2015-10-14 17:33           ` Sören Brinkmann
2015-10-14 17:33             ` Sören Brinkmann
2015-10-14 18:22             ` Moritz Fischer
2015-10-14 18:22               ` Moritz Fischer
2015-10-19  6:55       ` Linus Walleij
2015-10-19  6:53   ` Linus Walleij
2015-10-14 12:51 ` [PATCH 3/5] MIPS: dt: xilfpga: Add xilfpga device tree files Zubair Lutfullah Kakakhel
2015-10-14 12:51   ` Zubair Lutfullah Kakakhel
     [not found]   ` <1444827117-10939-4-git-send-email-Zubair.Kakakhel-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
2015-10-15  7:45     ` James Hogan
2015-10-15  7:45       ` James Hogan
2015-10-15  7:45       ` James Hogan
2015-10-14 12:51 ` [PATCH 4/5] MIPS: xilfpga: Add mipsfpga platform code Zubair Lutfullah Kakakhel
2015-10-14 12:51   ` Zubair Lutfullah Kakakhel
2015-10-15  8:11   ` James Hogan
2015-10-15  8:11     ` James Hogan
2015-10-21 14:35   ` Alban [this message]
2015-10-21 14:35     ` Alban
2015-10-14 12:51 ` [PATCH 5/5] MIPS: Add xilfpga defconfig Zubair Lutfullah Kakakhel
2015-10-14 12:51   ` Zubair Lutfullah Kakakhel
2015-10-15  8:34   ` James Hogan
2015-10-15  8:34     ` James Hogan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151021163502.44be785f@avionic-0020 \
    --to=albeu@free.fr \
    --cc=Zubair.Kakakhel@imgtec.com \
    --cc=devicetree@vger.kernel.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@linux-mips.org \
    --cc=ralf@linux-mips.org \
    --cc=robh+dt@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.