Re: [PATCH 00/03] gpio: Renesas R-Car GPIO driver update
Hi Laurent, On Thu, Mar 14, 2013 at 10:13 PM, Laurent Pinchart wrote: > Hi Magnus, > > On Thursday 14 March 2013 13:23:46 Magnus Damm wrote: >> On Wed, Mar 13, 2013 at 9:58 PM, Laurent Pinchart wrote: >> > On Wednesday 13 March 2013 20:32:03 Magnus Damm wrote: >> >> gpio: Renesas R-Car GPIO driver update >> >> >> >> [PATCH 01/03] gpio: Renesas R-Car GPIO driver V2 >> >> [PATCH 02/03] gpio: rcar: Use IRQCHIP_SET_TYPE_MASKED >> >> [PATCH 03/03] gpio: rcar: Make use of devm functions >> >> >> >> This series updates the R-Car GPIO driver with various >> >> changes kindly suggested by Laurent Pinchart. >> >> >> >> Patch [01/03] is a drop in replacement for V1 of the R-Car >> >> GPIO driver. The flag in patch [02/03] is kept out of [01/03] >> >> to avoid changing the behaviour of the driver between V1 and V2. >> >> Also, devm support in [03/03] is kept out of [01/03] to make >> >> sure back porting goes as smooth as possible. >> > >> > As I mentioned in a previous e-mail, all the devm_* functions used in >> > 03/03 have been available since 2.6.30. Do you really need to port that >> > driver to older kernels ? >> >> Well, it was earlier suggested to me that not using devm to begin with >> is a safe way forward for back porting. Also, as the individual patch >> shows, we save about 10 lines of code but add many more complex >> dependencies. >> >> Simon, do you have any recommendation how for us regarding devm and >> back porting? >> >> > Regarding 02/03, do you plan to squash it with 01/03 for the mainline >> > submission ? >> >> Not unless someone puts a gun to my head. =) Of course, if a single >> patch is really required then I will follow that, but I can't really >> see why when we have a nice versioning control system that can help >> our development in various ways. >> >> What is the reason behind you wanting to merge these patches? >> >> From my point of view, if any incremental patch was a bug fix then i >> would of course request to fold it in, but in this case these are >> feature patches that would be beneficial to keep as individual >> commits. Keeping them separate allows us to bisect and also makes >> partial back porting easier if needed. > > When submitting new drivers I usually try not to make the development history > visible to mainline. It brings little additional value (beside possibly making > backporting a bit easier, but in the devm_* case that shouldn't be a problem, > unless Simon thinks otherwise) but adds review complexity, as reviewers need > to validate the intermediate versions as well. More patches also mean more > potential bisection breakages. Huh, it seems that my point of view is the total opposite. I see that using incremental patches to show new development would make review _easier_. Perhaps that's not the case for most people. Regarding bisection, having features broken out actually allows us to bisect compared to a single commit. I see that as a feature. Of course the developer needs to make sure that the incremental changes do not cause any breakages, but if the developer can't do that then there are other more serious issues that need to be fixed IMO. > In this case (assuming there would be no backporting issue) there's no need > for mainline to see that we started with a version that didn't use devm_* and > then modified the code. I see no added value in that. So say that you write a driver and add say 8 features on top of that over time, wouldn't it make sense to share that information with other people? That way any party can bisect and locate issues that may come up. I find that useful regardless if the code is back ported or not. Anyway, with this particular driver it doesn't really matter since the complexity is very low. And now Simon has gotten back to us with updated information about back porting. I will pull it all together into a V3. Thanks, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 00/05] irqchip: Renesas INTC External IRQ pin v2 update
irqchip: Renesas INTC External IRQ pin v2 update [PATCH 01/05] irqchip: intc-irqpin: Whitespace fixes [PATCH 02/05] irqchip: intc-irqpin: Cache mapped IRQ [PATCH 03/05] irqchip: intc-irqpin: Add force comments [PATCH 04/05] irqchip: intc-irqpin: Make use of devm functions [PATCH 05/05] irqchip: intc-irqpin: GPL header for platform data These patches update the v1 of the INTC External IRQ pin driver in various ways based on feedback kindly received from: - Simon Horman - Kuninori Morimoto - Thomas Gleixner With the series applied I see the driver as "v2", but I prefer to keep these changes as incremental ones instead of redoing a single "v2" patch for the driver. The reason behind this is that there are on-going back porting efforts that are better dealt with using incremental patches. Signed-off-by: Magnus Damm --- Depends on: [PATCH] irqchip: Renesas INTC External IRQ pin driver drivers/irqchip/irq-renesas-intc-irqpin.c | 88 - include/linux/platform_data/irq-renesas-intc-irqpin.h | 19 +++ 2 files changed, 62 insertions(+), 45 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 01/05] irqchip: intc-irqpin: Whitespace fixes
From: Magnus Damm
Remove whitespace damage and add newline
between variables and code.
Signed-off-by: Magnus Damm
---
Thanks to Simon Horman and Thomas Gleixner for feedback.
drivers/irqchip/irq-renesas-intc-irqpin.c |8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
--- 0002/drivers/irqchip/irq-renesas-intc-irqpin.c
+++ work/drivers/irqchip/irq-renesas-intc-irqpin.c 2013-02-26
19:01:12.0 +0900
@@ -57,13 +57,13 @@ struct intc_irqpin_iomem {
unsigned long (*read)(void __iomem *iomem);
void (*write)(void __iomem *iomem, unsigned long data);
int width;
-};
+};
struct intc_irqpin_irq {
int hw_irq;
int irq;
struct intc_irqpin_priv *p;
-};
+};
struct intc_irqpin_priv {
struct intc_irqpin_iomem iomem[INTC_IRQPIN_REG_NR];
@@ -99,6 +99,7 @@ static inline unsigned long intc_irqpin_
int reg)
{
struct intc_irqpin_iomem *i = &p->iomem[reg];
+
return i->read(i->iomem);
}
@@ -106,6 +107,7 @@ static inline void intc_irqpin_write(str
int reg, unsigned long data)
{
struct intc_irqpin_iomem *i = &p->iomem[reg];
+
i->write(i->iomem, data);
}
@@ -405,7 +407,7 @@ static int intc_irqpin_probe(struct plat
dev_warn(&pdev->dev, "irq base mismatch (%d/%d)\n",
p->config.irq_base, k);
}
-
+
return 0;
err3:
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 02/05] irqchip: intc-irqpin: Cache mapped IRQ
From: Magnus Damm
Cache IRQ in domain_irq variable instead of
making use of irq_find_mapping(). While at it
rename the irq variable to requested_irq.
Signed-off-by: Magnus Damm
---
Thanks to Thomas Gleixner for this suggestion.
drivers/irqchip/irq-renesas-intc-irqpin.c | 30 +++--
1 file changed, 16 insertions(+), 14 deletions(-)
--- 0003/drivers/irqchip/irq-renesas-intc-irqpin.c
+++ work/drivers/irqchip/irq-renesas-intc-irqpin.c 2013-02-26
19:29:56.0 +0900
@@ -61,7 +61,8 @@ struct intc_irqpin_iomem {
struct intc_irqpin_irq {
int hw_irq;
- int irq;
+ int requested_irq;
+ int domain_irq;
struct intc_irqpin_priv *p;
};
@@ -171,8 +172,7 @@ static int intc_irqpin_set_sense(struct
static void intc_irqpin_dbg(struct intc_irqpin_irq *i, char *str)
{
dev_dbg(&i->p->pdev->dev, "%s (%d:%d:%d)\n",
- str, i->irq, i->hw_irq,
- irq_find_mapping(i->p->irq_domain, i->hw_irq));
+ str, i->requested_irq, i->hw_irq, i->domain_irq);
}
static void intc_irqpin_irq_enable(struct irq_data *d)
@@ -196,7 +196,7 @@ static void intc_irqpin_irq_disable(stru
static void intc_irqpin_irq_enable_force(struct irq_data *d)
{
struct intc_irqpin_priv *p = irq_data_get_irq_chip_data(d);
- int irq = p->irq[irqd_to_hwirq(d)].irq;
+ int irq = p->irq[irqd_to_hwirq(d)].requested_irq;
intc_irqpin_irq_enable(d);
irq_get_chip(irq)->irq_unmask(irq_get_irq_data(irq));
@@ -205,7 +205,7 @@ static void intc_irqpin_irq_enable_force
static void intc_irqpin_irq_disable_force(struct irq_data *d)
{
struct intc_irqpin_priv *p = irq_data_get_irq_chip_data(d);
- int irq = p->irq[irqd_to_hwirq(d)].irq;
+ int irq = p->irq[irqd_to_hwirq(d)].requested_irq;
irq_get_chip(irq)->irq_mask(irq_get_irq_data(irq));
intc_irqpin_irq_disable(d);
@@ -246,7 +246,7 @@ static irqreturn_t intc_irqpin_irq_handl
if (intc_irqpin_read(p, INTC_IRQPIN_REG_SOURCE) & bit) {
intc_irqpin_write(p, INTC_IRQPIN_REG_SOURCE, ~bit);
intc_irqpin_dbg(i, "demux2");
- generic_handle_irq(irq_find_mapping(p->irq_domain, i->hw_irq));
+ generic_handle_irq(i->domain_irq);
return IRQ_HANDLED;
}
return IRQ_NONE;
@@ -257,6 +257,9 @@ static int intc_irqpin_irq_domain_map(st
{
struct intc_irqpin_priv *p = h->host_data;
+ p->irq[hw].domain_irq = virq;
+ p->irq[hw].hw_irq = hw;
+
intc_irqpin_dbg(&p->irq[hw], "map");
irq_set_chip_data(virq, h->host_data);
irq_set_chip_and_handler(virq, &p->irq_chip, handle_level_irq);
@@ -314,9 +317,8 @@ static int intc_irqpin_probe(struct plat
if (!irq)
break;
- p->irq[k].hw_irq = k;
p->irq[k].p = p;
- p->irq[k].irq = irq->start;
+ p->irq[k].requested_irq = irq->start;
}
p->number_of_irqs = k;
@@ -389,7 +391,8 @@ static int intc_irqpin_probe(struct plat
/* request and set priority on interrupts one by one */
for (k = 0; k < p->number_of_irqs; k++) {
- if (request_irq(p->irq[k].irq, intc_irqpin_irq_handler,
+ if (request_irq(p->irq[k].requested_irq,
+ intc_irqpin_irq_handler,
0, name, &p->irq[k])) {
dev_err(&pdev->dev, "failed to request low IRQ\n");
ret = -ENOENT;
@@ -402,17 +405,16 @@ static int intc_irqpin_probe(struct plat
/* warn in case of mismatch if irq base is specified */
if (p->config.irq_base) {
- k = irq_find_mapping(p->irq_domain, 0);
- if (p->config.irq_base != k)
+ if (p->config.irq_base != p->irq[0].domain_irq)
dev_warn(&pdev->dev, "irq base mismatch (%d/%d)\n",
-p->config.irq_base, k);
+p->config.irq_base, p->irq[0].domain_irq);
}
return 0;
err3:
for (; k >= 0; k--)
- free_irq(p->irq[k - 1].irq, &p->irq[k - 1]);
+ free_irq(p->irq[k - 1].requested_irq, &p->irq[k - 1]);
irq_domain_remove(p->irq_domain);
err2:
@@ -430,7 +432,7 @@ static int intc_irqpin_remove(struct pla
int k;
for (k = 0; k < p->number_of_irqs; k++)
- free_irq(p->irq[k].irq, &p->irq[k]);
+ free_irq(p->irq[k].requested_irq, &p->irq[k]);
irq_domain_remove(p->irq_domain);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 03/05] irqchip: intc-irqpin: Add force comments
From: Magnus Damm Add comments to describe the special case for "force" versions of enable and disable functions. Signed-off-by: Magnus Damm --- Thanks to Thomas Gleixner for this suggestion. drivers/irqchip/irq-renesas-intc-irqpin.c |9 + 1 file changed, 9 insertions(+) --- 0006/drivers/irqchip/irq-renesas-intc-irqpin.c +++ work/drivers/irqchip/irq-renesas-intc-irqpin.c 2013-02-26 19:36:33.0 +0900 @@ -199,6 +199,11 @@ static void intc_irqpin_irq_enable_force int irq = p->irq[irqd_to_hwirq(d)].requested_irq; intc_irqpin_irq_enable(d); + + /* enable interrupt through parent interrupt controller, +* assumes non-shared interrupt with 1:1 mapping +* needed for busted IRQs on some SoCs like sh73a0 +*/ irq_get_chip(irq)->irq_unmask(irq_get_irq_data(irq)); } @@ -207,6 +212,10 @@ static void intc_irqpin_irq_disable_forc struct intc_irqpin_priv *p = irq_data_get_irq_chip_data(d); int irq = p->irq[irqd_to_hwirq(d)].requested_irq; + /* disable interrupt through parent interrupt controller, +* assumes non-shared interrupt with 1:1 mapping +* needed for busted IRQs on some SoCs like sh73a0 +*/ irq_get_chip(irq)->irq_mask(irq_get_irq_data(irq)); intc_irqpin_irq_disable(d); } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 04/05] irqchip: intc-irqpin: Make use of devm functions
From: Magnus Damm
Use devm_kzalloc(), devm_ioremap_nocache()
and devm_request_irq() to simplify error
handling.
Signed-off-by: Magnus Damm
---
Thanks to Kuninori Morimoto for this suggestion.
drivers/irqchip/irq-renesas-intc-irqpin.c | 41 +
1 file changed, 13 insertions(+), 28 deletions(-)
--- 0007/drivers/irqchip/irq-renesas-intc-irqpin.c
+++ work/drivers/irqchip/irq-renesas-intc-irqpin.c 2013-02-26
19:50:11.0 +0900
@@ -294,7 +294,7 @@ static int intc_irqpin_probe(struct plat
int ret;
int k;
- p = kzalloc(sizeof(*p), GFP_KERNEL);
+ p = devm_kzalloc(&pdev->dev, sizeof(*p), GFP_KERNEL);
if (!p) {
dev_err(&pdev->dev, "failed to allocate driver data\n");
ret = -ENOMEM;
@@ -316,7 +316,7 @@ static int intc_irqpin_probe(struct plat
if (!io[k]) {
dev_err(&pdev->dev, "not enough IOMEM resources\n");
ret = -EINVAL;
- goto err1;
+ goto err0;
}
}
@@ -334,7 +334,7 @@ static int intc_irqpin_probe(struct plat
if (p->number_of_irqs < 1) {
dev_err(&pdev->dev, "not enough IRQ resources\n");
ret = -EINVAL;
- goto err1;
+ goto err0;
}
/* ioremap IOMEM and setup read/write callbacks */
@@ -355,14 +355,15 @@ static int intc_irqpin_probe(struct plat
default:
dev_err(&pdev->dev, "IOMEM size mismatch\n");
ret = -EINVAL;
- goto err2;
+ goto err0;
}
- i->iomem = ioremap_nocache(io[k]->start, resource_size(io[k]));
+ i->iomem = devm_ioremap_nocache(&pdev->dev, io[k]->start,
+ resource_size(io[k]));
if (!i->iomem) {
dev_err(&pdev->dev, "failed to remap IOMEM\n");
ret = -ENXIO;
- goto err2;
+ goto err0;
}
}
@@ -395,17 +396,17 @@ static int intc_irqpin_probe(struct plat
if (!p->irq_domain) {
ret = -ENXIO;
dev_err(&pdev->dev, "cannot initialize irq domain\n");
- goto err2;
+ goto err0;
}
/* request and set priority on interrupts one by one */
for (k = 0; k < p->number_of_irqs; k++) {
- if (request_irq(p->irq[k].requested_irq,
- intc_irqpin_irq_handler,
- 0, name, &p->irq[k])) {
+ if (devm_request_irq(&pdev->dev, p->irq[k].requested_irq,
+intc_irqpin_irq_handler,
+0, name, &p->irq[k])) {
dev_err(&pdev->dev, "failed to request low IRQ\n");
ret = -ENOENT;
- goto err3;
+ goto err1;
}
intc_irqpin_mask_unmask_prio(p, k, 0);
}
@@ -421,16 +422,8 @@ static int intc_irqpin_probe(struct plat
return 0;
-err3:
- for (; k >= 0; k--)
- free_irq(p->irq[k - 1].requested_irq, &p->irq[k - 1]);
-
- irq_domain_remove(p->irq_domain);
-err2:
- for (k = 0; k < INTC_IRQPIN_REG_NR; k++)
- iounmap(p->iomem[k].iomem);
err1:
- kfree(p);
+ irq_domain_remove(p->irq_domain);
err0:
return ret;
}
@@ -438,17 +431,9 @@ err0:
static int intc_irqpin_remove(struct platform_device *pdev)
{
struct intc_irqpin_priv *p = platform_get_drvdata(pdev);
- int k;
-
- for (k = 0; k < p->number_of_irqs; k++)
- free_irq(p->irq[k].requested_irq, &p->irq[k]);
irq_domain_remove(p->irq_domain);
- for (k = 0; k < INTC_IRQPIN_REG_NR; k++)
- iounmap(p->iomem[k].iomem);
-
- kfree(p);
return 0;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 05/05] irqchip: intc-irqpin: GPL header for platform data
From: Magnus Damm Add GPL header to platform data include file. Signed-off-by: Magnus Damm --- Thanks to Kuninori Morimoto for this suggestion. include/linux/platform_data/irq-renesas-intc-irqpin.h | 19 + 1 file changed, 19 insertions(+) --- 0002/include/linux/platform_data/irq-renesas-intc-irqpin.h +++ work/include/linux/platform_data/irq-renesas-intc-irqpin.h 2013-02-26 19:57:03.0 +0900 @@ -1,3 +1,22 @@ +/* + * Renesas INTC External IRQ Pin Driver + * + * Copyright (C) 2013 Magnus Damm + * + * 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 + * + * 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. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + #ifndef __IRQ_RENESAS_INTC_IRQPIN_H__ #define __IRQ_RENESAS_INTC_IRQPIN_H__ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] gpio: em: Add Device Tree support
From: Magnus Damm
Update the Emma Mobile GPIO driver to add DT support.
The patch simply adds a two-cell xlate function and
updates the probe code to allow configuration via DT
using the "ngpios" property plus OF id in the same
style as gpio-mvebu.c. The code is also adjusted to
use postcore_initcall() to force early setup.
Signed-off-by: Magnus Damm
---
Written on top of:
[PATCH] gpio: em: Use irq_domain_add_simple() to fix runtime error
drivers/gpio/gpio-em.c | 45 ++---
1 file changed, 42 insertions(+), 3 deletions(-)
--- 0010/drivers/gpio/gpio-em.c
+++ work/drivers/gpio/gpio-em.c 2013-02-26 22:11:58.0 +0900
@@ -231,10 +231,12 @@ static int em_gio_irq_domain_map(struct
static struct irq_domain_ops em_gio_irq_domain_ops = {
.map= em_gio_irq_domain_map,
+ .xlate = irq_domain_xlate_twocell,
};
static int em_gio_probe(struct platform_device *pdev)
{
+ struct gpio_em_config pdata_dt;
struct gpio_em_config *pdata = pdev->dev.platform_data;
struct em_gio_priv *p;
struct resource *io[2], *irq[2];
@@ -259,8 +261,8 @@ static int em_gio_probe(struct platform_
irq[0] = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
irq[1] = platform_get_resource(pdev, IORESOURCE_IRQ, 1);
- if (!io[0] || !io[1] || !irq[0] || !irq[1] || !pdata) {
- dev_err(&pdev->dev, "missing IRQ, IOMEM or configuration\n");
+ if (!io[0] || !io[1] || !irq[0] || !irq[1]) {
+ dev_err(&pdev->dev, "missing IRQ or IOMEM\n");
ret = -EINVAL;
goto err1;
}
@@ -279,6 +281,25 @@ static int em_gio_probe(struct platform_
goto err2;
}
+ if (!pdata) {
+ memset(&pdata_dt, 0, sizeof(pdata_dt));
+ pdata = &pdata_dt;
+
+ if (of_property_read_u32(pdev->dev.of_node, "ngpios",
+&pdata->number_of_pins)) {
+ dev_err(&pdev->dev, "Missing ngpios OF property\n");
+ ret = -EINVAL;
+ goto err3;
+ }
+
+ ret = of_alias_get_id(pdev->dev.of_node, "gpio");
+ if (ret < 0) {
+ dev_err(&pdev->dev, "Couldn't get OF id\n");
+ goto err3;
+ }
+ pdata->gpio_base = ret * 32; /* 32 GPIOs per instance */
+ }
+
gpio_chip = &p->gpio_chip;
gpio_chip->direction_input = em_gio_direction_input;
gpio_chip->get = em_gio_get;
@@ -366,15 +387,33 @@ static int em_gio_remove(struct platform
return 0;
}
+static const struct of_device_id em_gio_dt_ids[] = {
+ { .compatible = "renesas,em-gio", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, em_gio_dt_ids);
+
static struct platform_driver em_gio_device_driver = {
.probe = em_gio_probe,
.remove = em_gio_remove,
.driver = {
.name = "em_gio",
+ .of_match_table = em_gio_dt_ids,
+ .owner = THIS_MODULE,
}
};
-module_platform_driver(em_gio_device_driver);
+static int __init em_gio_init(void)
+{
+ return platform_driver_register(&em_gio_device_driver);
+}
+postcore_initcall(em_gio_init);
+
+static void __exit em_gio_exit(void)
+{
+ platform_driver_unregister(&em_gio_device_driver);
+}
+module_exit(em_gio_exit);
MODULE_AUTHOR("Magnus Damm");
MODULE_DESCRIPTION("Renesas Emma Mobile GIO Driver");
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] irqchip: Renesas IRQC driver
From: Magnus Damm
This patch adds a driver for external IRQ pins connected
to the IRQC hardware block on recent SoCs from Renesas.
The IRQC hardware block is used together with more
recent ARM based SoCs using the GIC. As usual the GIC
requires external IRQ trigger setup somewhere else
which in this particular case happens to be IRQC.
This driver implements the glue code needed to configure
IRQ trigger and also handle mask/unmask and demux of
external IRQ pins hooked up from the IRQC to the GIC.
Tested on r8a73a4 but is designed to work with a wide
range of SoCs. The driver requires one GIC SPI per
external IRQ pin to operate. Each driver instance
will handle up to 32 external IRQ pins.
The SoCs using this driver are currently mainly used
together with regular platform devices so this driver
allows configuration via platform data to support things
like static interrupt base address. DT support will
be added incrementally in the not so distant future.
Signed-off-by: Magnus Damm
---
drivers/irqchip/Kconfig|4
drivers/irqchip/Makefile |1
drivers/irqchip/irq-renesas-irqc.c | 298
include/linux/platform_data/irq-renesas-irqc.h | 27 ++
4 files changed, 330 insertions(+)
--- 0002/drivers/irqchip/Kconfig
+++ work/drivers/irqchip/Kconfig2013-02-27 16:00:08.0 +0900
@@ -29,6 +29,10 @@ config RENESAS_INTC_IRQPIN
bool
select IRQ_DOMAIN
+config RENESAS_IRQC
+ bool
+ select IRQ_DOMAIN
+
config VERSATILE_FPGA_IRQ
bool
select IRQ_DOMAIN
--- 0002/drivers/irqchip/Makefile
+++ work/drivers/irqchip/Makefile 2013-02-27 16:00:08.0 +0900
@@ -6,4 +6,5 @@ obj-$(CONFIG_ARCH_SPEAR3XX) += spear-sh
obj-$(CONFIG_ARM_GIC) += irq-gic.o
obj-$(CONFIG_ARM_VIC) += irq-vic.o
obj-$(CONFIG_RENESAS_INTC_IRQPIN) += irq-renesas-intc-irqpin.o
+obj-$(CONFIG_RENESAS_IRQC) += irq-renesas-irqc.o
obj-$(CONFIG_VERSATILE_FPGA_IRQ) += irq-versatile-fpga.o
--- /dev/null
+++ work/drivers/irqchip/irq-renesas-irqc.c 2013-02-27 17:04:02.0
+0900
@@ -0,0 +1,298 @@
+/*
+ * Renesas IRQC Driver
+ *
+ * Copyright (C) 2013 Magnus Damm
+ *
+ * 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
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define IRQC_IRQ_MAX 32 /* maximum 32 interrupts per driver instance */
+
+#define IRQC_REQ_STS 0x00
+#define IRQC_EN_STS 0x04
+#define IRQC_EN_SET 0x08
+#define IRQC_INT_CPU_BASE(n) (0x000 + ((n) * 0x10))
+#define DETECT_STATUS 0x100
+#define IRQC_CONFIG(n) (0x180 + ((n) * 0x04))
+
+struct irqc_irq {
+ int hw_irq;
+ int requested_irq;
+ int domain_irq;
+ struct irqc_priv *p;
+};
+
+struct irqc_priv {
+ void __iomem *iomem;
+ void __iomem *cpu_int_base;
+ struct irqc_irq irq[IRQC_IRQ_MAX];
+ struct renesas_irqc_config config;
+ unsigned int number_of_irqs;
+ struct platform_device *pdev;
+ struct irq_chip irq_chip;
+ struct irq_domain *irq_domain;
+};
+
+static void irqc_dbg(struct irqc_irq *i, char *str)
+{
+ dev_dbg(&i->p->pdev->dev, "%s (%d:%d:%d)\n",
+ str, i->requested_irq, i->hw_irq, i->domain_irq);
+}
+
+static void irqc_irq_enable(struct irq_data *d)
+{
+ struct irqc_priv *p = irq_data_get_irq_chip_data(d);
+ int hw_irq = irqd_to_hwirq(d);
+
+ irqc_dbg(&p->irq[hw_irq], "enable");
+ iowrite32(BIT(hw_irq), p->cpu_int_base + IRQC_EN_SET);
+}
+
+static void irqc_irq_disable(struct irq_data *d)
+{
+ struct irqc_priv *p = irq_data_get_irq_chip_data(d);
+ int hw_irq = irqd_to_hwirq(d);
+
+ irqc_dbg(&p->irq[hw_irq], "disable");
+ iowrite32(BIT(hw_irq), p->cpu_int_base + IRQC_EN_STS);
+}
+
+#define INTC_IRQ_SENSE_VALID 0x10
+#define INTC_IRQ_SENSE(x) (x + INTC_IRQ_SENSE_VALID)
+
+static unsigned char irqc_sense[IRQ_TYPE_SENSE_MASK + 1] = {
+ [IRQ_TYPE_LEVEL_LOW] = INTC_IRQ_SENSE(0x01),
+ [IRQ_TYPE_LEVEL_HIGH] = INTC_IRQ_SENSE(0x02),
+ [IRQ_TYPE_EDGE_FALLING] = INTC_IRQ_SENSE(0x04), /* Synchronous */
+ [IRQ_TYPE_EDGE_RISIN
Re: [PATCH] irqchip: Renesas INTC External IRQ pin driver
On Wed, Feb 27, 2013 at 5:23 PM, Paul Mundt wrote: > On Mon, Feb 18, 2013 at 11:28:34PM +0900, Magnus Damm wrote: >> From: Magnus Damm >> >> This patch adds a driver for external IRQ pins connected >> to the INTC block on recent SoCs from Renesas. >> > So how exactly does this interact with the existing sh_intc code? Or is > there some reason why you have opted to bypass it in order to implement a > simplified reduced-functionality version of INTC support focused only on > external pins? If both are used together this is going to be a nightmare > for locking, and it's also non-obvious how the IRQ domains on both sides > will interact. > > This needs a lot more explanation. Recent GIC-based SoCs do not make use of INTC for any on-chip I/O devices. This driver is meant to be used as a layer between the actual IRQ pin and the GIC. Anything else needs the full driver. The existing non-DT INTC driver can happily coexist with this driver like it does in the case of sh73a0 here: [PATCH 02/03] ARM: shmobile: INTC External IRQ pin driver on sh73a0 The driver is not meant to be used with INTC-only based systems like sh7372 and the SH architecture. I would be very happy if someone could get their shit together and fix up DT support for the common INTC code. This has not happened yet though. So if you know anyone with time to spare then feel free to suggest them to work together with Iwamatsu-san to get the DT version of the code reviewed together with Linaro. Thanks, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] irqchip: Renesas INTC External IRQ pin driver
On Wed, Feb 27, 2013 at 5:52 PM, Paul Mundt wrote: > On Wed, Feb 27, 2013 at 05:35:51PM +0900, Magnus Damm wrote: >> On Wed, Feb 27, 2013 at 5:23 PM, Paul Mundt wrote: >> > So how exactly does this interact with the existing sh_intc code? Or is >> > there some reason why you have opted to bypass it in order to implement a >> > simplified reduced-functionality version of INTC support focused only on >> > external pins? If both are used together this is going to be a nightmare >> > for locking, and it's also non-obvious how the IRQ domains on both sides >> > will interact. >> > >> > This needs a lot more explanation. >> >> Recent GIC-based SoCs do not make use of INTC for any on-chip I/O >> devices. This driver is meant to be used as a layer between the actual >> IRQ pin and the GIC. Anything else needs the full driver. The existing >> non-DT INTC driver can happily coexist with this driver like it does >> in the case of sh73a0 here: >> >> [PATCH 02/03] ARM: shmobile: INTC External IRQ pin driver on sh73a0 >> > Ok, thanks for clarifying. > > I suppose the main concern is how quickly this will simply turn in to a > deviated partial implementation of the full driver as newer SoCs begin > deviating from your simplified case, and we basically end up > reimplementing sh_intc anyways. As you know, the INTC code that you are referring to is a full interrupt controller designed to work directly with CPU cores like SH and ARM. Newer ARM cores like Cortex-A9 all include the GIC both for IPI purpose in case of SMP and they also implement SPI functionality for I/O device interrupts. So the need for vendor-specific interrupt controller IP is almost down to nothing these days. With that in mind I do really doubt that we end up reimplementing sh_intc. The upcoming designs that I am aware of do not change their external IRQ pin hardware to force us to update this driver. What happens after that I'm afraid I can't say. =) >> The driver is not meant to be used with INTC-only based systems like >> sh7372 and the SH architecture. I would be very happy if someone could >> get their shit together and fix up DT support for the common INTC >> code. This has not happened yet though. So if you know anyone with >> time to spare then feel free to suggest them to work together with >> Iwamatsu-san to get the DT version of the code reviewed together with >> Linaro. >> > I haven't heard or seen anything new on that in some time, so I assumed > the work had stalled. I'm not sure why there wasn't more effort put in to > DT support for the INTC code rather than simply coming up with a > temporary bypass shim, and I'm not sure why you think this work is > blocked by anyone (unless you're just referring to a general lack of > resources). I mainly wrote this driver to support the r8a7779 SoC which can't be driven by the existing INTC code base. During development I decided to try to share the driver between multiple GIC-based SoCs like r8a7779 and sh73a0. The reason behind trying to share this driver between multiple SoCs is to remove SoC-specific hacks that can't be dealt with by the existing INTC code. > In any event, I'm not sure what the best option for the interim is. I > suppose we can merge the irqchips until the INTC stuff catches up, but > then we are probaby going to run in to a situation where they either have > to co-exist, or the irqchips are removed and the sh_intc code has to > carry a compat shim to deal with those DT bindings. Neither of those > options seem particularly appealing to me. I don't really understand why you would want to use a generic table driven driver when you have the possibility of using a hardware-specific driver. I suppose you are wondering why no one seems to be working on INTC DT support at this point. The truth is that we got very little feedback and development support with interrupts in general last autumn and on top of that our developers went their own way instead of following advice. Anyway, I do understand your worry about what happens if the hardware starts to deviate, but judging by the future devices that I've seen we should be ok for a while. Beyond that no one can tell. Thanks, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio: em: Add Device Tree support
Hi Dmitry,
Thanks for your feedback!
On Wed, Feb 27, 2013 at 7:41 AM, Dmitry Torokhov
wrote:
> Hi Magnus,
>
> On Tue, Feb 26, 2013 at 10:26:23PM +0900, Magnus Damm wrote:
>> From: Magnus Damm
>>
>> Update the Emma Mobile GPIO driver to add DT support.
>>
>
> ...
>
>> @@ -366,15 +387,33 @@ static int em_gio_remove(struct platform
>> return 0;
>> }
>>
>
> #ifdef CONFIG_OF here? No need to have extra aliases in modules if OF
> support is not enabled (or is entire ARM arch now enables device tree?).
>
>> +static const struct of_device_id em_gio_dt_ids[] = {
>> + { .compatible = "renesas,em-gio", },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, em_gio_dt_ids);
>> +
I suppose we could sprinkle a couple of #ifdefs across the code, but I
have to say that I'm not that fond of #ifdefs in general. So if it was
up to me only then I would aim at having exactly zero #ifdefs in my
drivers at the expense of slightly larger binaries in some cases.
Are you interested in hacking on EMEV2 in general? I assume you're
busy, but I could try to locate a developer board for you if you'd
like.
Cheers,
/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] irqchip: Renesas INTC External IRQ pin driver
On Tue, Feb 19, 2013 at 10:03 AM, Simon Horman wrote: > On Mon, Feb 18, 2013 at 11:28:34PM +0900, Magnus Damm wrote: >> From: Magnus Damm >> >> This patch adds a driver for external IRQ pins connected >> to the INTC block on recent SoCs from Renesas. >> >> The INTC hardware block usually contains a rather wide >> range of features ranging from external IRQ pin handling >> to legacy interrupt controller support. On older SoCs >> the INTC is used as a general purpose interrupt controller >> both for external IRQ pins and on-chip devices. >> >> On more recent ARM based SoCs with Cortex-A9 the main >> interrupt controller is the GIC, but IRQ trigger setup >> still need to happen in the INTC hardware block. >> >> This driver implements the glue code needed to configure >> IRQ trigger and also handle mask/unmask and demux of >> external IRQ pins hooked up from the INTC to the GIC. >> >> Tested on sh73a0 and r8a7779. The hardware varies quite >> a bit with SoC model, for instance register width and >> bitfield widths vary wildly. The driver requires one GIC >> SPI per external IRQ pin to operate. Each driver instance >> will handle up to 8 external IRQ pins. >> >> The SoCs using this driver are currently mainly used >> together with regular platform devices so this driver >> allows configuration via platform data to support things >> like static interrupt base address. DT support will >> be added incrementally in the not so distant future. >> >> Signed-off-by: Magnus Damm > > Hi Magnus, Hi all, > > I do not expect this code to go through the renesas tree. However, in > order to provide a basis for work on renesas SoCs I have added this patch > to the topic/intc-external-irq topic branch in the reneas tree on > kernel.org and merged it into topic/all+next. > > In other words, I am not picking this series up to merge it or add it to > linux-next, rather I am storing it for reference. > > > In the course of adding the branch I noticed a 3 whitespace warnings > from git. I have highlighted them below. Thanks Simon. I will fix those up in V2! / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] irqchip: Renesas INTC External IRQ pin driver
Hi Morimoto-san,
On Tue, Feb 19, 2013 at 10:04 AM, Kuninori Morimoto
wrote:
>
> Hi Magnus
>
> Thank you for this patch.
> Small comment from me :)
Sure, thanks!
>> +struct intc_irqpin_priv {
>> + struct intc_irqpin_iomem iomem[INTC_IRQPIN_REG_NR];
>> + struct intc_irqpin_irq irq[INTC_IRQPIN_MAX];
>> + struct renesas_intc_irqpin_config config;
>> + unsigned int number_of_irqs;
>> + struct platform_device *pdev;
>> + struct irq_chip irq_chip;
>> + struct irq_domain *irq_domain;
>> +};
>
> (snip)
>
>> +static DEFINE_RAW_SPINLOCK(intc_irqpin_lock); /* only used by slow path */
>> +
>> +static void intc_irqpin_read_modify_write(struct intc_irqpin_priv *p,
>> + int reg, int shift,
>> + int width, int value)
>> +{
>> + unsigned long flags;
>> + unsigned long tmp;
>> +
>> + raw_spin_lock_irqsave(&intc_irqpin_lock, flags);
>> +
>> + tmp = intc_irqpin_read(p, reg);
>> + tmp &= ~(((1 << width) - 1) << shift);
>> + tmp |= value << shift;
>> + intc_irqpin_write(p, reg, tmp);
>> +
>> + raw_spin_unlock_irqrestore(&intc_irqpin_lock, flags);
>> +}
>
> It is possible to include this spin lock into priv ?
> This local static spin lock seems not wrong, but looks strange ?
Please see this comment (that you snipped out above):
+/* INTC external IRQ PIN hardware register access:
+ *
+ * SENSE is read-write 32-bit with 2-bits or 4-bits per IRQ (*)
+ * PRIO is read-write 32-bit with 4-bits per IRQ (**)
+ * SOURCE is read-only 32-bit or 8-bit with 1-bit per IRQ (***)
+ * MASK is write-only 32-bit or 8-bit with 1-bit per IRQ (***)
+ * CLEAR is write-only 32-bit or 8-bit with 1-bit per IRQ (***)
+ *
+ * (*) May be accessed by more than one driver instance - lock needed
+ * (**) Read-modify-write access by one driver instance - lock needed
+ * (***) Accessed by one driver instance only - no locking needed
+ */
Basically, the lock is used for SENSE and PRIO. SENSE may be shared
between multiple driver instances, so a lock in ->priv won't be
enough. PRIO may be locked using ->priv but both SENSE and PRIV are in
the slow path so I decided to handle both using a global lock.
>> +static int intc_irqpin_probe(struct platform_device *pdev)
>> +{
>> + struct renesas_intc_irqpin_config *pdata = pdev->dev.platform_data;
>> + struct intc_irqpin_priv *p;
>> + struct intc_irqpin_iomem *i;
>> + struct resource *io[INTC_IRQPIN_REG_NR];
>> + struct resource *irq;
>> + struct irq_chip *irq_chip;
>> + void (*enable_fn)(struct irq_data *d);
>> + void (*disable_fn)(struct irq_data *d);
>> + const char *name = dev_name(&pdev->dev);
>> + int ret;
>> + int k;
>> +
>> + p = kzalloc(sizeof(*p), GFP_KERNEL);
>
> can you use devm_kzalloc() ?
> devm_ioremap_nocache() or devm_request_and_ioremap()
> Can you use devm_request_irq()
> if you used devm_, you can remove kfree() / free_irq() / iounmap() here
I somehow knew that this would come up. Will give devm a go in V2 -
unless someone tells me such a change is going to make the driver more
painful to back port to LTSI-3.4.
>> --- /dev/null
>> +++ work/include/linux/platform_data/irq-renesas-intc-irqpin.h
>> 2013-02-18 18:28:24.0 +0900
>> @@ -0,0 +1,10 @@
>> +#ifndef __IRQ_RENESAS_INTC_IRQPIN_H__
>> +#define __IRQ_RENESAS_INTC_IRQPIN_H__
>
> GPL license signage ?
Let me check how other files in include/linux/platform_data/ handles
this. I will follow the majority.
Thanks,
/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] irqchip: Renesas INTC External IRQ pin driver
Hi Thomas,
Thanks for your help with the review!
On Tue, Feb 19, 2013 at 7:11 PM, Thomas Gleixner wrote:
> Magnus,
>
> On Mon, 18 Feb 2013, Magnus Damm wrote:
>
>> +static inline unsigned long intc_irqpin_read(struct intc_irqpin_priv *p,
>> + int reg)
>> +{
>> + struct intc_irqpin_iomem *i = &p->iomem[reg];
>
> Newline between variable and code please.
Yes, I agree, will fix. I have been criticized for adding too many
newlines in the past, so I guess this is a good sign that I can flip
the other way now!
>> + return i->read(i->iomem);
>> +}
>> +
>> +static inline void intc_irqpin_write(struct intc_irqpin_priv *p,
>> + int reg, unsigned long data)
>> +{
>> + struct intc_irqpin_iomem *i = &p->iomem[reg];
>> + i->write(i->iomem, data);
>> +}
>> +
>> +static inline unsigned long intc_irqpin_hwirq_mask(struct intc_irqpin_priv
>> *p,
>> +int reg, int hw_irq)
>> +{
>> + return BIT((p->iomem[reg].width - 1) - hw_irq);
>> +}
>> +
>> +static inline void intc_irqpin_irq_write_hwirq(struct intc_irqpin_priv *p,
>> +int reg, int hw_irq)
>> +{
>> + intc_irqpin_write(p, reg, intc_irqpin_hwirq_mask(p, reg, hw_irq));
>> +}
>> +
>> +static DEFINE_RAW_SPINLOCK(intc_irqpin_lock); /* only used by slow path */
>
> Shouldn't the lock be part of struct intc_irqpin_priv ?
Good idea, but I need to lock access to the SENSE register against
multiple driver instances. This is not the case for PRIO. But because
both PRIO and SENSE are accessed in the slow path I decided to be lazy
and use the same way of locking to reduce the code size.
>> +static void intc_irqpin_read_modify_write(struct intc_irqpin_priv *p,
>> + int reg, int shift,
>> + int width, int value)
>> +{
>> + unsigned long flags;
>> + unsigned long tmp;
>> +
>> + raw_spin_lock_irqsave(&intc_irqpin_lock, flags);
>> +static void intc_irqpin_dbg(struct intc_irqpin_irq *i, char *str)
>> +{
>> + dev_dbg(&i->p->pdev->dev, "%s (%d:%d:%d)\n",
>> + str, i->irq, i->hw_irq,
>> + irq_find_mapping(i->p->irq_domain, i->hw_irq));
>
> Do you really want to do a lookup for each debug invocation?
Uhm, maybe no. I need to check if this compiles out in case of DEBUG=n.
>> +}
>> +
>> +static void intc_irqpin_irq_enable(struct irq_data *d)
>> +{
>> + struct intc_irqpin_priv *p = irq_data_get_irq_chip_data(d);
>> + int hw_irq = irqd_to_hwirq(d);
>> +
>> + intc_irqpin_dbg(&p->irq[hw_irq], "enable");
>> + intc_irqpin_irq_write_hwirq(p, INTC_IRQPIN_REG_CLEAR, hw_irq);
>> +}
>> +
>> +static void intc_irqpin_irq_disable(struct irq_data *d)
>> +{
>> + struct intc_irqpin_priv *p = irq_data_get_irq_chip_data(d);
>> + int hw_irq = irqd_to_hwirq(d);
>> +
>> + intc_irqpin_dbg(&p->irq[hw_irq], "disable");
>> + intc_irqpin_irq_write_hwirq(p, INTC_IRQPIN_REG_MASK, hw_irq);
>> +}
>> +
>> +static void intc_irqpin_irq_enable_force(struct irq_data *d)
>> +{
>> + struct intc_irqpin_priv *p = irq_data_get_irq_chip_data(d);
>> + int irq = p->irq[irqd_to_hwirq(d)].irq;
>> +
>> + intc_irqpin_irq_enable(d);
>> + irq_get_chip(irq)->irq_unmask(irq_get_irq_data(irq));
>> +}
>> +
>> +static void intc_irqpin_irq_disable_force(struct irq_data *d)
>> +{
>> + struct intc_irqpin_priv *p = irq_data_get_irq_chip_data(d);
>> + int irq = p->irq[irqd_to_hwirq(d)].irq;
>> +
>> + irq_get_chip(irq)->irq_mask(irq_get_irq_data(irq));
>> + intc_irqpin_irq_disable(d);
>
> Hmm. If I understand that code correctly, then the _force functions
> are (un)masking another interrupt line. But this happens without
> holding irq_desc[irq]->lock. Looks unsafe at least and if correct
> wants a big fat comment.
On some SoCs the masking for some IRQs seems busted, and the only sane
way to work around that is to (un)mask the parent interrupt. The
parent happens to be the GIC. I will look into how to add locking.
>> +}
>> +
>> +#define INTC_IRQ_SENSE_VALID 0x10
>> +#define INTC_IRQ_SENSE(x) (x + INTC_IRQ_SENSE_VALID)
>> +
>> +static unsigned char intc_irqpin_sense[IRQ_TYPE_SENSE_MASK + 1] =
[PATCH] gpio: em: Make use of devm functions
From: Magnus Damm
Update the Emma Mobile GPIO driver to make use of devm
functions. This simplifies the error handling and makes
the code more compact.
Signed-off-by: Magnus Damm
---
Written on top of:
[PATCH] gpio: em: Add Device Tree support
drivers/gpio/gpio-em.c | 53 +---
1 file changed, 19 insertions(+), 34 deletions(-)
--- 0002/drivers/gpio/gpio-em.c
+++ work/drivers/gpio/gpio-em.c 2013-03-13 18:52:14.0 +0900
@@ -245,7 +245,7 @@ static int em_gio_probe(struct platform_
const char *name = dev_name(&pdev->dev);
int ret;
- p = kzalloc(sizeof(*p), GFP_KERNEL);
+ p = devm_kzalloc(&pdev->dev, sizeof(*p), GFP_KERNEL);
if (!p) {
dev_err(&pdev->dev, "failed to allocate driver data\n");
ret = -ENOMEM;
@@ -264,21 +264,23 @@ static int em_gio_probe(struct platform_
if (!io[0] || !io[1] || !irq[0] || !irq[1]) {
dev_err(&pdev->dev, "missing IRQ or IOMEM\n");
ret = -EINVAL;
- goto err1;
+ goto err0;
}
- p->base0 = ioremap_nocache(io[0]->start, resource_size(io[0]));
+ p->base0 = devm_ioremap_nocache(&pdev->dev, io[0]->start,
+ resource_size(io[0]));
if (!p->base0) {
dev_err(&pdev->dev, "failed to remap low I/O memory\n");
ret = -ENXIO;
- goto err1;
+ goto err0;
}
- p->base1 = ioremap_nocache(io[1]->start, resource_size(io[1]));
+ p->base1 = devm_ioremap_nocache(&pdev->dev, io[1]->start,
+ resource_size(io[1]));
if (!p->base1) {
dev_err(&pdev->dev, "failed to remap high I/O memory\n");
ret = -ENXIO;
- goto err2;
+ goto err0;
}
if (!pdata) {
@@ -289,13 +291,13 @@ static int em_gio_probe(struct platform_
&pdata->number_of_pins)) {
dev_err(&pdev->dev, "Missing ngpios OF property\n");
ret = -EINVAL;
- goto err3;
+ goto err0;
}
ret = of_alias_get_id(pdev->dev.of_node, "gpio");
if (ret < 0) {
dev_err(&pdev->dev, "Couldn't get OF id\n");
- goto err3;
+ goto err0;
}
pdata->gpio_base = ret * 32; /* 32 GPIOs per instance */
}
@@ -327,40 +329,32 @@ static int em_gio_probe(struct platform_
if (!p->irq_domain) {
ret = -ENXIO;
dev_err(&pdev->dev, "cannot initialize irq domain\n");
- goto err3;
+ goto err0;
}
- if (request_irq(irq[0]->start, em_gio_irq_handler, 0, name, p)) {
+ if (devm_request_irq(&pdev->dev, irq[0]->start,
+em_gio_irq_handler, 0, name, p)) {
dev_err(&pdev->dev, "failed to request low IRQ\n");
ret = -ENOENT;
- goto err4;
+ goto err1;
}
- if (request_irq(irq[1]->start, em_gio_irq_handler, 0, name, p)) {
+ if (devm_request_irq(&pdev->dev, irq[1]->start,
+em_gio_irq_handler, 0, name, p)) {
dev_err(&pdev->dev, "failed to request high IRQ\n");
ret = -ENOENT;
- goto err5;
+ goto err1;
}
ret = gpiochip_add(gpio_chip);
if (ret) {
dev_err(&pdev->dev, "failed to add GPIO controller\n");
- goto err6;
+ goto err1;
}
return 0;
-err6:
- free_irq(irq[1]->start, pdev);
-err5:
- free_irq(irq[0]->start, pdev);
-err4:
- irq_domain_remove(p->irq_domain);
-err3:
- iounmap(p->base1);
-err2:
- iounmap(p->base0);
err1:
- kfree(p);
+ irq_domain_remove(p->irq_domain);
err0:
return ret;
}
@@ -368,22 +362,13 @@ err0:
static int em_gio_remove(struct platform_device *pdev)
{
struct em_gio_priv *p = platform_get_drvdata(pdev);
- struct resource *irq[2];
int ret;
ret = gpiochip_remove(&p->gpio_chip);
if (ret)
return ret;
- irq[0] = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
- irq[1] = platform_get_resource(pdev, IORESOURCE_IRQ, 1);
-
- free_irq(irq[1]->start, pdev);
- free_irq(irq[0]->start, pdev);
irq_domain_remove(p->irq_domain);
- iounmap(p->base1);
- iounmap(p->base0);
- kfree(p);
return 0;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 00/03] gpio: Renesas R-Car GPIO driver update
gpio: Renesas R-Car GPIO driver update [PATCH 01/03] gpio: Renesas R-Car GPIO driver V2 [PATCH 02/03] gpio: rcar: Use IRQCHIP_SET_TYPE_MASKED [PATCH 03/03] gpio: rcar: Make use of devm functions This series updates the R-Car GPIO driver with various changes kindly suggested by Laurent Pinchart. Patch [01/03] is a drop in replacement for V1 of the R-Car GPIO driver. The flag in patch [02/03] is kept out of [01/03] to avoid changing the behaviour of the driver between V1 and V2. Also, devm support in [03/03] is kept out of [01/03] to make sure back porting goes as smooth as possible. Signed-off-by: Magnus Damm --- drivers/gpio/Kconfig|6 drivers/gpio/Makefile |1 drivers/gpio/gpio-rcar.c| 417 +-- include/linux/platform_data/gpio-rcar.h | 25 + 4 files changed, 427 insertions(+), 22 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 01/03] gpio: Renesas R-Car GPIO driver V2
From: Magnus Damm
This patch is V2 of a GPIO driver for the R-Car series of
SoCs from Renesas. This driver is designed to be reusable
between multiple SoCs that share the same basic building block,
but so far it has only been used on R-Car H1 (r8a7779).
Each driver instance handles 32 GPIOs with individually
maskable IRQs. The driver operates on a single I/O memory
range and the 32 GPIOs are hooked up a single interrupt.
In the case of R-Car H1 either external IRQ pins or GPIOs
with interrupts can be used for on-board interupts. For
external IRQs 4 pins are supported, and in the case of GPIO
there are 202 GPIOS as 202 interrupts hooked up via 6 driver
instances and to the GIC and the Cortex-A9 Quad.
At this point this driver is interfacing as a regular
platform device driver. In the future DT support will be
submitted as an incremental feature patch.
Signed-off-by: Magnus Damm
---
Changes since V1:
- Update based on most suggestions from review by Laurent, thanks!
Please note that in V2 the driver is reworked to reduce the number of
lines and includes minor changes related to headers, printouts and
data types. In V2 the register access order is kept the same as in V1,
and to make that happen IRQCHIP_SET_TYPE_MASKED is omitted. The interface
to the Linux kernel is unchanged except the use of dev_dbg() instead of
pr_debug(). So from a hardware point of view V2 is a drop in replacement
for V1 and there should be no additional dependencies since devm is
kept as a separate patch as well. All this to make back porting easier.
drivers/gpio/Kconfig|6
drivers/gpio/Makefile |1
drivers/gpio/gpio-rcar.c| 383 +++
include/linux/platform_data/gpio-rcar.h | 25 ++
4 files changed, 415 insertions(+)
--- 0001/drivers/gpio/Kconfig
+++ work/drivers/gpio/Kconfig 2013-03-13 19:39:44.0 +0900
@@ -201,6 +201,12 @@ config GPIO_PXA
help
Say yes here to support the PXA GPIO device
+config GPIO_RCAR
+ tristate "Renesas R-Car GPIO"
+ depends on ARM
+ help
+ Say yes here to support GPIO on Renesas R-Car SoCs.
+
config GPIO_SPEAR_SPICS
bool "ST SPEAr13xx SPI Chip Select as GPIO support"
depends on PLAT_SPEAR
--- 0001/drivers/gpio/Makefile
+++ work/drivers/gpio/Makefile 2013-03-13 19:39:44.0 +0900
@@ -56,6 +56,7 @@ obj-$(CONFIG_GPIO_PL061) += gpio-pl061.o
obj-$(CONFIG_GPIO_PXA) += gpio-pxa.o
obj-$(CONFIG_GPIO_RC5T583) += gpio-rc5t583.o
obj-$(CONFIG_GPIO_RDC321X) += gpio-rdc321x.o
+obj-$(CONFIG_GPIO_RCAR)+= gpio-rcar.o
obj-$(CONFIG_PLAT_SAMSUNG) += gpio-samsung.o
obj-$(CONFIG_ARCH_SA1100) += gpio-sa1100.o
obj-$(CONFIG_GPIO_SCH) += gpio-sch.o
--- /dev/null
+++ work/drivers/gpio/gpio-rcar.c 2013-03-13 19:41:35.0 +0900
@@ -0,0 +1,383 @@
+/*
+ * Renesas R-Car GPIO Support
+ *
+ * Copyright (C) 2013 Magnus Damm
+ *
+ * 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
+ *
+ * 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
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct gpio_rcar_priv {
+ void __iomem *base;
+ spinlock_t lock;
+ struct gpio_rcar_config config;
+ struct platform_device *pdev;
+ struct gpio_chip gpio_chip;
+ struct irq_chip irq_chip;
+ struct irq_domain *irq_domain;
+};
+
+#define IOINTSEL 0x00
+#define INOUTSEL 0x04
+#define OUTDT 0x08
+#define INDT 0x0c
+#define INTDT 0x10
+#define INTCLR 0x14
+#define INTMSK 0x18
+#define MSKCLR 0x1c
+#define POSNEG 0x20
+#define EDGLEVEL 0x24
+#define FILONOFF 0x28
+
+static inline u32 gpio_rcar_read(struct gpio_rcar_priv *p, int offs)
+{
+ return ioread32(p->base + offs);
+}
+
+static inline void gpio_rcar_write(struct gpio_rcar_priv *p, int offs,
+ u32 value)
+{
+ iowrite32(value, p->base + offs);
+}
+
+static void gpio_rcar_modify_bit(struct gpio_rcar_priv *p, int offs,
+int bit, bool value)
+{
+ u32 tmp = gpio_rcar_read(p, offs);
+
+ if (value)
+ tmp |= BIT(bit);
+ else
+ tmp &= ~BIT(bit);
+
+ gpio_rcar_write(p, offs, tmp);
+}
+
+static void gpio_rcar_irq_disable(struct irq_data *d)
+{
+ struct gpio_rcar_priv *p = irq_data_get_irq_chip_data(d);
+
+ gpio_rcar_write(p, INTMSK, ~BIT(irqd_to_hwirq(d)));
+}
+
+static void gpio_rcar_irq_enable(struct
[PATCH 02/03] gpio: rcar: Use IRQCHIP_SET_TYPE_MASKED
From: Magnus Damm Set the flag IRQCHIP_SET_TYPE_MASKED in gpio-rcar.c to force mask of the interrupt while configuring trigger settings. Signed-off-by: Magnus Damm --- drivers/gpio/gpio-rcar.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- 0004/drivers/gpio/gpio-rcar.c +++ work/drivers/gpio/gpio-rcar.c 2013-03-13 19:42:56.0 +0900 @@ -301,7 +301,7 @@ static int gpio_rcar_probe(struct platfo irq_chip->irq_enable = gpio_rcar_irq_enable; irq_chip->irq_disable = gpio_rcar_irq_disable; irq_chip->irq_set_type = gpio_rcar_irq_set_type; - irq_chip->flags = IRQCHIP_SKIP_SET_WAKE; + irq_chip->flags = IRQCHIP_SKIP_SET_WAKE | IRQCHIP_SET_TYPE_MASKED; p->irq_domain = irq_domain_add_simple(pdev->dev.of_node, p->config.number_of_pins, -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 03/03] gpio: rcar: Make use of devm functions
From: Magnus Damm
Update the gpio-rcar.c driver to make use of devm
functions. This simplifies the error handling and
makes the code more compact.
Signed-off-by: Magnus Damm
---
drivers/gpio/gpio-rcar.c | 32 +++-
1 file changed, 11 insertions(+), 21 deletions(-)
--- 0005/drivers/gpio/gpio-rcar.c
+++ work/drivers/gpio/gpio-rcar.c 2013-03-13 19:47:17.0 +0900
@@ -252,7 +252,7 @@ static int gpio_rcar_probe(struct platfo
const char *name = dev_name(&pdev->dev);
int ret;
- p = kzalloc(sizeof(*p), GFP_KERNEL);
+ p = devm_kzalloc(&pdev->dev, sizeof(*p), GFP_KERNEL);
if (!p) {
dev_err(&pdev->dev, "failed to allocate driver data\n");
ret = -ENOMEM;
@@ -273,14 +273,15 @@ static int gpio_rcar_probe(struct platfo
if (!io || !irq) {
dev_err(&pdev->dev, "missing IRQ or IOMEM\n");
ret = -EINVAL;
- goto err1;
+ goto err0;
}
- p->base = ioremap_nocache(io->start, resource_size(io));
+ p->base = devm_ioremap_nocache(&pdev->dev, io->start,
+ resource_size(io));
if (!p->base) {
dev_err(&pdev->dev, "failed to remap I/O memory\n");
ret = -ENXIO;
- goto err1;
+ goto err0;
}
gpio_chip = &p->gpio_chip;
@@ -310,19 +311,20 @@ static int gpio_rcar_probe(struct platfo
if (!p->irq_domain) {
ret = -ENXIO;
dev_err(&pdev->dev, "cannot initialize irq domain\n");
- goto err2;
+ goto err1;
}
- if (request_irq(irq->start, gpio_rcar_irq_handler, 0, name, p)) {
+ if (devm_request_irq(&pdev->dev, irq->start,
+gpio_rcar_irq_handler, 0, name, p)) {
dev_err(&pdev->dev, "failed to request IRQ\n");
ret = -ENOENT;
- goto err3;
+ goto err1;
}
ret = gpiochip_add(gpio_chip);
if (ret) {
dev_err(&pdev->dev, "failed to add GPIO controller\n");
- goto err4;
+ goto err1;
}
dev_info(&pdev->dev, "driving %d GPIOs\n", p->config.number_of_pins);
@@ -337,14 +339,8 @@ static int gpio_rcar_probe(struct platfo
return 0;
-err4:
- free_irq(irq->start, p);
-err3:
- irq_domain_remove(p->irq_domain);
-err2:
- iounmap(p->base);
err1:
- kfree(p);
+ irq_domain_remove(p->irq_domain);
err0:
return ret;
}
@@ -352,19 +348,13 @@ err0:
static int gpio_rcar_remove(struct platform_device *pdev)
{
struct gpio_rcar_priv *p = platform_get_drvdata(pdev);
- struct resource *irq;
int ret;
ret = gpiochip_remove(&p->gpio_chip);
if (ret)
return ret;
- irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-
- free_irq(irq->start, p);
irq_domain_remove(p->irq_domain);
- iounmap(p->base);
- kfree(p);
return 0;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio: Renesas R-Car GPIO driver
Hi Linus,
On Thu, Mar 14, 2013 at 2:20 AM, Linus Walleij wrote:
> On Tue, Mar 5, 2013 at 1:32 AM, Magnus Damm wrote:
>
>> +static int gpio_rcar_irq_set_type(struct irq_data *d, unsigned int type)
>> +{
>> + struct gpio_rcar_priv *p = irq_data_get_irq_chip_data(d);
>> + unsigned int hwirq = irqd_to_hwirq(d);
>> +
>> + pr_debug("gpio: sense irq = %d, type = %d\n", hwirq, type);
>> +
>> + switch (type & IRQ_TYPE_SENSE_MASK) {
>> + case IRQ_TYPE_LEVEL_HIGH:
>> + gpio_rcar_config_interrupt_input_mode(p, hwirq, true, true);
>> + break;
>> + case IRQ_TYPE_LEVEL_LOW:
>> + gpio_rcar_config_interrupt_input_mode(p, hwirq, false, true);
>> + break;
>> + case IRQ_TYPE_EDGE_RISING:
>> + gpio_rcar_config_interrupt_input_mode(p, hwirq, true, false);
>> + break;
>> + case IRQ_TYPE_EDGE_FALLING:
>> + gpio_rcar_config_interrupt_input_mode(p, hwirq, false,
>> false);
>> + break;
>
> It is quite common to request IRQ on *both* rising and falling edges,
> what happens then?
>
> See
> IRQ_TYPE_EDGE_RISING= 0x0001,
> IRQ_TYPE_EDGE_FALLING = 0x0002,
> IRQ_TYPE_EDGE_BOTH = (IRQ_TYPE_EDGE_FALLING |
> IRQ_TYPE_EDGE_RISING),
>
> I guess then you get just rising edge...
I may be wrong, but the switch above does AND with IRQ_TYPE_SENSE_MASK
which makes me believe that the BOTH case will result in -EINVAL.
The -EINVAL is IMO correct here since the most basic version of this
GPIO block (used in r8a7779) does not suport BOTH edges. I've noticed
that newer versions of that GPIO hardware block has additional
registers where this can be selected. My plan is to submit an
incremental patch for such feature in the not so distant future.
> The rest I think is already covered by Laurent's comments.
Ok, thanks for your help!
/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 01/03] gpio: Renesas R-Car GPIO driver V2
On Wed, Mar 13, 2013 at 10:14 PM, Laurent Pinchart
wrote:
> Hi Magnus,
>
> Thanks for the patch.
>
> I've reviewed the result of squashing the 3 patches together, I just have one
> comment.
>
> On Wednesday 13 March 2013 20:32:13 Magnus Damm wrote:
>> From: Magnus Damm
>>
>> This patch is V2 of a GPIO driver for the R-Car series of
>> SoCs from Renesas. This driver is designed to be reusable
>> between multiple SoCs that share the same basic building block,
>> but so far it has only been used on R-Car H1 (r8a7779).
>>
>> Each driver instance handles 32 GPIOs with individually
>> maskable IRQs. The driver operates on a single I/O memory
>> range and the 32 GPIOs are hooked up a single interrupt.
>>
>> In the case of R-Car H1 either external IRQ pins or GPIOs
>> with interrupts can be used for on-board interupts. For
>> external IRQs 4 pins are supported, and in the case of GPIO
>> there are 202 GPIOS as 202 interrupts hooked up via 6 driver
>> instances and to the GIC and the Cortex-A9 Quad.
>>
>> At this point this driver is interfacing as a regular
>> platform device driver. In the future DT support will be
>> submitted as an incremental feature patch.
>>
>> Signed-off-by: Magnus Damm
>> ---
>>
>> Changes since V1:
>> - Update based on most suggestions from review by Laurent, thanks!
>>
>> Please note that in V2 the driver is reworked to reduce the number of
>> lines and includes minor changes related to headers, printouts and
>> data types. In V2 the register access order is kept the same as in V1,
>> and to make that happen IRQCHIP_SET_TYPE_MASKED is omitted. The interface
>> to the Linux kernel is unchanged except the use of dev_dbg() instead of
>> pr_debug(). So from a hardware point of view V2 is a drop in replacement
>> for V1 and there should be no additional dependencies since devm is
>> kept as a separate patch as well. All this to make back porting easier.
>>
>> drivers/gpio/Kconfig|6
>> drivers/gpio/Makefile |1
>> drivers/gpio/gpio-rcar.c| 383
>> include/linux/platform_data/gpio-rcar.h | 25 ++
>> 4 files changed, 415 insertions(+)
>
> [snip]
>
>> --- /dev/null
>> +++ work/drivers/gpio/gpio-rcar.c 2013-03-13 19:41:35.0 +0900
>> @@ -0,0 +1,383 @@
>> +/*
>> + * Renesas R-Car GPIO Support
>> + *
>> + * Copyright (C) 2013 Magnus Damm
>> + *
>> + * 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
>> + *
>> + * 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.
>> + */
>
> [snip]
>
>> +static int gpio_rcar_irq_domain_map(struct irq_domain *h, unsigned int
>> virq,
>> + irq_hw_number_t hw)
>> +{
>> + struct gpio_rcar_priv *p = h->host_data;
>> +
>> + dev_dbg(&p->pdev->dev, "map hw irq = %d, virq = %d\n", (int)hw, virq);
>> +
>> + irq_set_chip_data(virq, h->host_data);
>> + irq_set_chip_and_handler(virq, &p->irq_chip, handle_level_irq);
>> + set_irq_flags(virq, IRQF_VALID); /* kill me now */
>
> What is that comment about ?
I seem to have a habit of putting that comment in all IRQ code that I
write for ARM. See these threads for more info:
https://lkml.org/lkml/2013/2/19/246
http://lists.infradead.org/pipermail/linux-arm-kernel/2010-January/008795.html
Cheers,
/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 00/03] gpio: Renesas R-Car GPIO driver update
Hi Laurent, Simon, On Wed, Mar 13, 2013 at 9:58 PM, Laurent Pinchart wrote: > Hi Magnus, > > Thanks for the patches. Thanks for your review! > On Wednesday 13 March 2013 20:32:03 Magnus Damm wrote: >> gpio: Renesas R-Car GPIO driver update >> >> [PATCH 01/03] gpio: Renesas R-Car GPIO driver V2 >> [PATCH 02/03] gpio: rcar: Use IRQCHIP_SET_TYPE_MASKED >> [PATCH 03/03] gpio: rcar: Make use of devm functions >> >> This series updates the R-Car GPIO driver with various >> changes kindly suggested by Laurent Pinchart. >> >> Patch [01/03] is a drop in replacement for V1 of the R-Car >> GPIO driver. The flag in patch [02/03] is kept out of [01/03] >> to avoid changing the behaviour of the driver between V1 and V2. >> Also, devm support in [03/03] is kept out of [01/03] to make >> sure back porting goes as smooth as possible. > > As I mentioned in a previous e-mail, all the devm_* functions used in 03/03 > have been available since 2.6.30. Do you really need to port that driver to > older kernels ? Well, it was earlier suggested to me that not using devm to begin with is a safe way forward for back porting. Also, as the individual patch shows, we save about 10 lines of code but add many more complex dependencies. Simon, do you have any recommendation how for us regarding devm and back porting? > Regarding 02/03, do you plan to squash it with 01/03 for the mainline > submission ? Not unless someone puts a gun to my head. =) Of course, if a single patch is really required then I will follow that, but I can't really see why when we have a nice versioning control system that can help our development in various ways. What is the reason behind you wanting to merge these patches? >From my point of view, if any incremental patch was a bug fix then i would of course request to fold it in, but in this case these are feature patches that would be beneficial to keep as individual commits. Keeping them separate allows us to bisect and also makes partial back porting easier if needed. Thanks, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio: em: Fix checking return value of irq_alloc_descs
On Fri, Aug 31, 2012 at 9:05 AM, Linus Walleij wrote: > On Tue, Aug 28, 2012 at 4:30 AM, Axel Lin wrote: > >> irq_alloc_descs() returns negative error code on failure. >> >> Signed-off-by: Axel Lin > > Magnuis can I have your ACK on this? Yes, of course! I never disagree with bug fixes =) Acked-by: Magnus Damm Thanks a lot to both Axel and you! / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Query] CPUFreq: Does these machines have separate clock domains for CPUs?
Hi Viresh, On Fri, Aug 30, 2013 at 4:33 PM, Viresh Kumar wrote: > On 30 August 2013 12:18, Magnus Damm wrote: >> Hi Viresh, >> >> On Thu, Aug 29, 2013 at 7:15 PM, Viresh Kumar >> wrote: >>> Hi, >>> >>> I have been doing some CPUFreq cleanup work and >>> wanted to know if the below mentioned machines have separate >>> clock domains for their CPUs or all share the same domain? >>> >>> So, that we can use some generic routines for these drivers which >>> would eventually do: >>> >>> cpumask_setall(policy->cpus); >>> >>> And I wanted to make sure that this doesn't break them.. :) >>> >>> .. >>> >>> The drivers are: >> ... >>> drivers/cpufreq/sh-cpufreq.c >> ... >> >> The above SH cpufreq driver seems to be written with SMP in mind, but >> I would say SMP is a very rare case for SH. So I believe it can be >> considered as UP-only at this point. If Paul disagrees I'm quite sure >> he will tell us. > > Okay.. The problem isn't really SMP but different clock domains for CPUs > in a SMP system.. > > So, even if we have a SMP SH machine, will it have same clock line for > all CPUs? Yeah, I understand your question but I'm afraid that I don't know the answer myself. > I will go with the change anyway.. Good plan. Thanks for cleaning up the cpufreq bits. Cheers, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/3] ARM: shmobile: kzm9d: Use common clock framework
On Tue, Oct 8, 2013 at 2:34 PM, wrote:
> Use common clock framework version of clock
> drivers/clk/shmobile/clk-emev2.c
> instead of sh-clkfwk version
> arch/arm/mach-shmobile/clock-emev2.c
> when it is configured as a part of multi-platform.
>
> Signed-off-by: Takashi Yoshii
> ---
> arch/arm/mach-shmobile/board-kzm9d-reference.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-shmobile/board-kzm9d-reference.c
> b/arch/arm/mach-shmobile/board-kzm9d-reference.c
> index 054d8d5..853003c 100644
> --- a/arch/arm/mach-shmobile/board-kzm9d-reference.c
> +++ b/arch/arm/mach-shmobile/board-kzm9d-reference.c
> @@ -20,15 +20,14 @@
>
> #include
> #include
> +#include
> #include
> #include
> #include
>
> static void __init kzm9d_add_standard_devices(void)
> {
> - if (!IS_ENABLED(CONFIG_COMMON_CLK))
> - emev2_clock_init();
> -
> + of_clk_init(NULL);
Hi Yoshii-san,
Thanks, this looks very good. It is the correct way now when
renesas-devel-20131008 includes the following series:
[PATCH 00/05] ARM: shmobile: KZM9D Multiplatform update
Acked-by: Magnus Damm
Cheers,
/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/3] ARM: shmobile: emev2: Add clock tree description in DT
On Tue, Oct 8, 2013 at 2:33 PM, wrote: > Add minimum clock tree description to .dts file. > This provides same set of clocks as current sh-clkfwk version .c > code does. > > Signed-off-by: Takashi Yoshii > --- > arch/arm/boot/dts/emev2.dtsi | 84 > > 1 file changed, 84 insertions(+) Looking good, thanks Yoshii-san! Acked-by: Magnus Damm -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/3] clk: emev2: Add support for emev2 SMU clocks with DT
On Tue, Oct 8, 2013 at 2:32 PM, wrote: > Device tree clock binding document for EMMA Mobile EV2 SMU, > And Common clock framework based implementation of it. > Following nodes are defined to describe clock tree. > - renesas,emev2-smu > - renesas,emev2-smu-clkdiv > - renesas,emev2-smu-gclk > > These bindings are designed manually based on > 19UH0037EJ1000_SMU : System Management Unit User's Manual > > So far, reparent is not implemented, and is fixed to index #0. > Clock tree description is not included, and should be provided > by device-tree. > > Signed-off-by: Takashi Yoshii > --- > .../devicetree/bindings/clock/emev2-clock.txt | 98 +++ > drivers/clk/Makefile | 1 + > drivers/clk/shmobile/Makefile | 3 + > drivers/clk/shmobile/clk-emev2.c | 104 > + > 4 files changed, 206 insertions(+) Thanks for cleaning up the Makefile bits, Yoshii-san. This patch and the bindings look fine to me from a SoC point of view. Using these together with the topology information in emev2.dtsi makes it possible for us to use CCF and multiplatform as expected on the EMEV2 SoC. Acked-by: Magnus Damm -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/3] ARM: shmobile: kzm9d: Use common clock framework
Hi Simon, On Wed, Oct 9, 2013 at 12:40 PM, Simon Horman wrote: > On Tue, Oct 08, 2013 at 02:34:03PM +0900, takas...@ops.dti.ne.jp wrote: >> Use common clock framework version of clock >> drivers/clk/shmobile/clk-emev2.c >> instead of sh-clkfwk version >> arch/arm/mach-shmobile/clock-emev2.c >> when it is configured as a part of multi-platform. >> >> Signed-off-by: Takashi Yoshii > > Thanks. > > I plan to add this patch to a new topic branch, > topic/emev2-common-clock, in the renesas tree and > queue it up from there for inclusion in mainline > if/when the first patch of this series is accepted > by Mike Turquette. Thanks for picking up patches, Simon. I think you can simply merge this patch after the following series: [PATCH 00/05] ARM: shmobile: KZM9D Multiplatform update There are no build time dependencies on patch 1 and 2 so this patch can be merged independently. Regarding run-time operation, the multiplatform series above makes KZM9D DT reference only build for multiplatform, and in such case CCF is required. So if you want to keep KZM9D DT reference working until Mike Turquette accepts the CCF bits, then I recommend you to wait with "[PATCH 00/05] ARM: shmobile: KZM9D Multiplatform update" until all EMEV2 CCF bits have been merged. Another way is to merge "[PATCH 00/05] ARM: shmobile: KZM9D Multiplatform update" before the EMEV2 CCF bits, but if so you may as well merge this patch as well IMO. This multiplatform-update-series-merge-before-CCF plan will result in untestable KZM9D DT reference until EMEV2 CCF is merged. Either way is fine with me. Cheers, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/3] ARM: shmobile: kzm9d: Use common clock framework
Hi Simon, On Wed, Oct 9, 2013 at 3:54 PM, Simon Horman wrote: > On Wed, Oct 09, 2013 at 01:59:46PM +0900, Magnus Damm wrote: >> Hi Simon, >> >> On Wed, Oct 9, 2013 at 12:40 PM, Simon Horman wrote: >> > On Tue, Oct 08, 2013 at 02:34:03PM +0900, takas...@ops.dti.ne.jp wrote: >> >> Use common clock framework version of clock >> >> drivers/clk/shmobile/clk-emev2.c >> >> instead of sh-clkfwk version >> >> arch/arm/mach-shmobile/clock-emev2.c >> >> when it is configured as a part of multi-platform. >> >> >> >> Signed-off-by: Takashi Yoshii >> > >> > Thanks. >> > >> > I plan to add this patch to a new topic branch, >> > topic/emev2-common-clock, in the renesas tree and >> > queue it up from there for inclusion in mainline >> > if/when the first patch of this series is accepted >> > by Mike Turquette. >> >> Thanks for picking up patches, Simon. >> >> I think you can simply merge this patch after the following series: >> >> [PATCH 00/05] ARM: shmobile: KZM9D Multiplatform update > > This is already queued up. Yes, I know, thanks for that. I tried to explain the dependency in the 00/00 cover letter but I will try to be more clear next time! >> There are no build time dependencies on patch 1 and 2 so this patch >> can be merged independently. Regarding run-time operation, the >> multiplatform series above makes KZM9D DT reference only build for >> multiplatform, and in such case CCF is required. >> >> So if you want to keep KZM9D DT reference working until Mike Turquette >> accepts the CCF bits, then I recommend you to wait with "[PATCH 00/05] >> ARM: shmobile: KZM9D Multiplatform update" until all EMEV2 CCF bits >> have been merged. >> >> Another way is to merge "[PATCH 00/05] ARM: shmobile: KZM9D >> Multiplatform update" before the EMEV2 CCF bits, but if so you may as >> well merge this patch as well IMO. This >> multiplatform-update-series-merge-before-CCF plan will result in >> untestable KZM9D DT reference until EMEV2 CCF is merged. Either way is >> fine with me. > > I am mainly concerned that the bindings may change before > they are finally merged. And I thought it would be nice to avoid > having to fix up the usage of the bindings if they change. Sure, but the code in this patch looks like it simply starts DT CCF probing regardless of what the bindings look like. So I don't think you will have to worry about actual bindings detail. > But I'm happy to just queue-up patches 2 and 3 of this series > now if you prefer. Well, I meant patch 3 only. Patch 1 and 2 are tied together by dependencies. =) / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Query] CPUFreq: Does these machines have separate clock domains for CPUs?
Hi Viresh, On Thu, Aug 29, 2013 at 7:15 PM, Viresh Kumar wrote: > Hi, > > I have been doing some CPUFreq cleanup work and > wanted to know if the below mentioned machines have separate > clock domains for their CPUs or all share the same domain? > > So, that we can use some generic routines for these drivers which > would eventually do: > > cpumask_setall(policy->cpus); > > And I wanted to make sure that this doesn't break them.. :) > > .. > > The drivers are: ... > drivers/cpufreq/sh-cpufreq.c ... The above SH cpufreq driver seems to be written with SMP in mind, but I would say SMP is a very rare case for SH. So I believe it can be considered as UP-only at this point. If Paul disagrees I'm quite sure he will tell us. Cheers, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH/RFC] clocksource: Consolidate SH and ARM mach-shmobile Kconfig bits
From: Magnus Damm Based on request from Olof Johansson, move ARM mach-shmobile clocksource entries to drivers/clocksource/Kconfig, and at the same time make the SH architecture use these entries. Kconfig entries for CMT, MTU2, TMU and STI are with this patch now kept in one place. The SH SYS_SUPPORTS_nnn bits are dropped to simplye let the user select the appropriate ones using Kconfig instead of trying to filter available timers based on SoC. Known issues: 1) Not yet wrappend in menu/endmenu, populates the root driver menu -> proposal: Create separate patch to wrap all the clocksource entries? 2) Should the entries be sorted? -> proposal: Ignore cosmetic bits 3) What is correct dependency for multiplatform and single platform? -> proposal: Perhaps we should simply default to no? 4) Is single patch across 2 architectures and drivers/clocksource OK? -> proposal: Keep it like this if possible Not-yet-signed-off-by: Magnus Damm --- arch/arm/mach-shmobile/Kconfig | 19 -- arch/sh/Kconfig| 53 drivers/clocksource/Kconfig| 25 ++ 3 files changed, 25 insertions(+), 72 deletions(-) --- 0004/arch/arm/mach-shmobile/Kconfig +++ work/arch/arm/mach-shmobile/Kconfig 2013-09-26 07:51:06.0 +0900 @@ -307,25 +307,6 @@ config SHMOBILE_TIMER_HZ SH-Mobile systems using a 32768 Hz RCLK for clock events may want to select a HZ value such as 128 that can evenly divide RCLK. A HZ value that does not divide evenly may cause timer drift. - -config SH_TIMER_CMT - bool "CMT timer driver" - default y - help - This enables build of the CMT timer driver. - -config SH_TIMER_TMU - bool "TMU timer driver" - default y - help - This enables build of the TMU timer driver. - -config EM_TIMER_STI - bool "STI timer driver" - default y - help - This enables build of the STI timer driver. - endmenu endif --- 0001/arch/sh/Kconfig +++ work/arch/sh/Kconfig2013-09-26 08:00:49.0 +0900 @@ -121,15 +121,6 @@ config SYS_SUPPORTS_NUMA config SYS_SUPPORTS_PCI bool -config SYS_SUPPORTS_CMT - bool - -config SYS_SUPPORTS_MTU2 - bool - -config SYS_SUPPORTS_TMU - bool - config STACKTRACE_SUPPORT def_bool y @@ -189,14 +180,12 @@ config CPU_SH3 bool select CPU_HAS_INTEVT select CPU_HAS_SR_RB - select SYS_SUPPORTS_TMU config CPU_SH4 bool select CPU_HAS_INTEVT select CPU_HAS_SR_RB select CPU_HAS_FPU if !CPU_SH4AL_DSP - select SYS_SUPPORTS_TMU select SYS_SUPPORTS_HUGETLBFS if MMU config CPU_SH4A @@ -211,7 +200,6 @@ config CPU_SH4AL_DSP config CPU_SH5 bool select CPU_HAS_FPU - select SYS_SUPPORTS_TMU select SYS_SUPPORTS_HUGETLBFS if MMU config CPU_SHX2 @@ -248,7 +236,6 @@ choice config CPU_SUBTYPE_SH7619 bool "Support SH7619 processor" select CPU_SH2 - select SYS_SUPPORTS_CMT # SH-2A Processor Support @@ -256,50 +243,38 @@ config CPU_SUBTYPE_SH7201 bool "Support SH7201 processor" select CPU_SH2A select CPU_HAS_FPU - select SYS_SUPPORTS_MTU2 config CPU_SUBTYPE_SH7203 bool "Support SH7203 processor" select CPU_SH2A select CPU_HAS_FPU - select SYS_SUPPORTS_CMT - select SYS_SUPPORTS_MTU2 select ARCH_WANT_OPTIONAL_GPIOLIB select PINCTRL config CPU_SUBTYPE_SH7206 bool "Support SH7206 processor" select CPU_SH2A - select SYS_SUPPORTS_CMT - select SYS_SUPPORTS_MTU2 config CPU_SUBTYPE_SH7263 bool "Support SH7263 processor" select CPU_SH2A select CPU_HAS_FPU - select SYS_SUPPORTS_CMT - select SYS_SUPPORTS_MTU2 config CPU_SUBTYPE_SH7264 bool "Support SH7264 processor" select CPU_SH2A select CPU_HAS_FPU - select SYS_SUPPORTS_CMT - select SYS_SUPPORTS_MTU2 select PINCTRL config CPU_SUBTYPE_SH7269 bool "Support SH7269 processor" select CPU_SH2A select CPU_HAS_FPU - select SYS_SUPPORTS_CMT - select SYS_SUPPORTS_MTU2 select PINCTRL config CPU_SUBTYPE_MXG bool "Support MX-G processor" select CPU_SH2A - select SYS_SUPPORTS_MTU2 help Select MX-G if running on an R8A03022BG part. @@ -352,7 +327,6 @@ config CPU_SUBTYPE_SH7720 bool "Support SH7720 processor" select CPU_SH3 select CPU_HAS_DSP - select SYS_SUPPORTS_CMT select ARCH_WANT_OPTIONAL_GPIOLIB select USB_ARCH_HAS_OHCI select USB_OHCI_SH if USB_OHCI_HCD @@ -364,7 +338,6 @@ config CPU_SUBTYPE_SH7721 bool "Support SH7721 pro
Re: [PATCH 2/4] ARM: shmobile: r8a7790: add I2C support in Device Tree mode
Hi Guennadi,
On Thu, Sep 26, 2013 at 7:10 AM, Guennadi Liakhovetski
wrote:
> Hi Laurent
>
> On Wed, 25 Sep 2013, Laurent Pinchart wrote:
>
>> Hi Guennadi,
>>
>> Thank you for the patch.
>>
>> On Monday 09 September 2013 18:03:54 Guennadi Liakhovetski wrote:
>> > This patch adds clocks and clock lookup entries for the four I2C
>> > controllers on r8a7790 and respective Device Tree nodes.
>> >
>> > Signed-off-by: Guennadi Liakhovetski
>> > ---
>> > arch/arm/boot/dts/r8a7790.dtsi | 36
>> > +
>> > arch/arm/mach-shmobile/clock-r8a7790.c | 10
>> > 2 files changed, 46 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/arch/arm/boot/dts/r8a7790.dtsi
>> > b/arch/arm/boot/dts/r8a7790.dtsi
>> > index 885f9f4..a5021112 100644
>> > --- a/arch/arm/boot/dts/r8a7790.dtsi
>> > +++ b/arch/arm/boot/dts/r8a7790.dtsi
>> > @@ -127,6 +127,42 @@
>> > interrupts = <0 0 4>, <0 1 4>, <0 2 4>, <0 3 4>;
>> > };
>> >
>> > + i2c0: i2c@e6508000 {
>> > + #address-cells = <1>;
>> > + #size-cells = <0>;
>> > + compatible = "renesas,i2c-rcar-h2";
>> > + reg = <0 0xe6508000 0 0x40>;
>> > + interrupt-parent = <&gic>;
>> > + interrupts = <0 287 0x4>;
>>
>> Shouldn't you add state = "disabled" to all I2C controllers in order not to
>> enable the unused controllers by default ?
>
> It would be logical, yes, and I seem to remember having discussed this
> earlier with someone (with Magnus, IIRC), and the outcome was, that all
> Renesas .dtsi files so far define all I2C directly in enabled mode and
> that it's intentional, so, I just followed this pattern here. You can
> indeed check in other .dtsi files - all seem to do exactly the same.
Uhm, I think you mix up platform device use case and DT use case. In
the platform device case we define all devices by default, but that's
not how it should be for the DT case. In the case of DT then default
should most likely be "disabled". Please follow the direction of
Laurent.
Also, I mentioned this 25 times before already so once more cannot
hurt: "renesas,i2c-rcar-h2" needs to be replaced with something more
standard.
/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] ARM: shmobile: r8a7790: add I2C support in Device Tree mode
Hi Guennadi,
On Thu, Sep 26, 2013 at 12:07 AM, Guennadi Liakhovetski
wrote:
> Hi Magnus
>
> On Thu, 26 Sep 2013, Magnus Damm wrote:
>
>> Hi Guennadi,
>>
>> On Thu, Sep 26, 2013 at 7:10 AM, Guennadi Liakhovetski
>> wrote:
>> > Hi Laurent
>> >
>> > On Wed, 25 Sep 2013, Laurent Pinchart wrote:
>> >
>> >> Hi Guennadi,
>> >>
>> >> Thank you for the patch.
>> >>
>> >> On Monday 09 September 2013 18:03:54 Guennadi Liakhovetski wrote:
>> >> > This patch adds clocks and clock lookup entries for the four I2C
>> >> > controllers on r8a7790 and respective Device Tree nodes.
>> >> >
>> >> > Signed-off-by: Guennadi Liakhovetski
>> >> > ---
>> >> > arch/arm/boot/dts/r8a7790.dtsi | 36
>> >> > +
>> >> > arch/arm/mach-shmobile/clock-r8a7790.c | 10
>> >> > 2 files changed, 46 insertions(+), 0 deletions(-)
>> >> >
>> >> > diff --git a/arch/arm/boot/dts/r8a7790.dtsi
>> >> > b/arch/arm/boot/dts/r8a7790.dtsi
>> >> > index 885f9f4..a5021112 100644
>> >> > --- a/arch/arm/boot/dts/r8a7790.dtsi
>> >> > +++ b/arch/arm/boot/dts/r8a7790.dtsi
>> >> > @@ -127,6 +127,42 @@
>> >> > interrupts = <0 0 4>, <0 1 4>, <0 2 4>, <0 3 4>;
>> >> > };
>> >> >
>> >> > + i2c0: i2c@e6508000 {
>> >> > + #address-cells = <1>;
>> >> > + #size-cells = <0>;
>> >> > + compatible = "renesas,i2c-rcar-h2";
>> >> > + reg = <0 0xe6508000 0 0x40>;
>> >> > + interrupt-parent = <&gic>;
>> >> > + interrupts = <0 287 0x4>;
>> >>
>> >> Shouldn't you add state = "disabled" to all I2C controllers in order not
>> >> to
>> >> enable the unused controllers by default ?
>> >
>> > It would be logical, yes, and I seem to remember having discussed this
>> > earlier with someone (with Magnus, IIRC), and the outcome was, that all
>> > Renesas .dtsi files so far define all I2C directly in enabled mode and
>> > that it's intentional, so, I just followed this pattern here. You can
>> > indeed check in other .dtsi files - all seem to do exactly the same.
>>
>> Uhm, I think you mix up platform device use case and DT use case.
>
> I'm sure I don't.
Ok, but it sure sounds like that.
>> In
>> the platform device case we define all devices by default, but that's
>> not how it should be for the DT case. In the case of DT then default
>> should most likely be "disabled". Please follow the direction of
>> Laurent.
>
> To put the record straight - originally I suggested to use status
> "disabled" for mmc devices in .dtsi, before that the status property
> wasn't used in Renesas .dts(i) files, and that's when all I2C nodes were
> already present in .dtsi in enabled state.
Regarding "disabled", using that in a coherent way probably makes
sense. As for MMC, despite my efforts the DT binding development
turned out far from perfect. So during that time the good ideas
proposed may have accidentally been shot down together with the rest,
my apologies for that.
Now, would it be possible to make sure that all mach-shmobile I2C
descriptions follow the same style? I would like the SoCs to be
supported in the same way, not randomly - both then it comes to
compatbile string format and if "disabled" is used or not.
>> Also, I mentioned this 25 times before already so once more cannot
>> hurt: "renesas,i2c-rcar-h2" needs to be replaced with something more
>> standard.
>
> I didn't count, but yes, it has been discussed and that's been fixed in v2
> of my i2c patches. Now that I've got comments to the ARM patches I can
> also update them with a correct compatibility string.
Good, thanks.
/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/6] clocksource: em_sti: convert to clk_prepare/unprepare
On Tue, Sep 24, 2013 at 1:09 PM, wrote: > From: Shinya Kuribayashi > > Add calls to clk_prepare and unprepare so that EMMA Mobile EV2 can > migrate to the common clock framework. > > Signed-off-by: Shinya Kuribayashi > --- > drivers/clocksource/em_sti.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Looking good, thanks! Acked-by: Magnus Damm -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/6] serial8250-em: convert to clk_prepare/unprepare
On Tue, Sep 24, 2013 at 1:10 PM, wrote: > From: Shinya Kuribayashi > > Add calls to clk_prepare and unprepare so that EMMA Mobile EV2 can > migrate to the common clock framework. > > Signed-off-by: Shinya Kuribayashi > [takashi.yoshii...@renesas.com: edited for conflicts] > Signed-off-by: Takashi Yoshii > --- > drivers/tty/serial/8250/8250_em.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) This looks fine to me, thanks! Acked-by: Magnus Damm -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/6] clk: emev2: Add support for emev2 SMU clocks with DT
On Tue, Sep 24, 2013 at 1:15 PM, wrote: > Common clock framework version of emev2 clock support. > smu_clkdiv and smu_gclk are handled. > So far, reparent is not implemented, and is fixed to index #0. > SMU and small numbers of clocks are described in emev2.dtsi. > > That function and numbers of clocks are equivalent to current > sh-clkfwk version. It is just enough to run kzm9d-reference. > > Signed-off-by: Takashi Yoshii > --- > arch/arm/boot/dts/emev2.dtsi | 84 +++ > drivers/clk/Makefile | 2 + > drivers/clk/shmobile/Makefile| 5 ++ > drivers/clk/shmobile/clk-emev2.c | 104 > +++ > 4 files changed, 195 insertions(+) > create mode 100644 drivers/clk/shmobile/Makefile > create mode 100644 drivers/clk/shmobile/clk-emev2.c Hi Yoshii-san, Thanks for your efforts on this. I'm very pleased to see that you describe the clock topology using DT. In general I think your patch looks fine, but I have some comment related to the multiplatform integration, please see below. > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile > index 7b11106..3e64ac4 100644 > --- a/drivers/clk/Makefile > +++ b/drivers/clk/Makefile > @@ -32,6 +32,8 @@ obj-$(CONFIG_ARCH_VT8500) += clk-vt8500.o > obj-$(CONFIG_ARCH_ZYNQ)+= zynq/ > obj-$(CONFIG_ARCH_TEGRA) += tegra/ > obj-$(CONFIG_PLAT_SAMSUNG) += samsung/ > +obj-$(CONFIG_ARCH_SHMOBILE)+= shmobile/ > +obj-$(CONFIG_ARCH_SHMOBILE_MULTI) += shmobile/ Here I believe it is enough that you only use CONFIG_ARCH_SHMOBILE_MULTI. Building common clocks to coexist with the old legacy board code does not make any sense IMO. If you think it makes sense for some reason, please explain why. =) > diff --git a/drivers/clk/shmobile/Makefile b/drivers/clk/shmobile/Makefile > new file mode 100644 > index 000..6a26eb6 > --- /dev/null > +++ b/drivers/clk/shmobile/Makefile > @@ -0,0 +1,5 @@ > +ifeq ($(CONFIG_COMMON_CLK), y) > +obj-$(CONFIG_ARCH_EMEV2) += clk-emev2.o > +endif I don't think you would need the above ifeq/endif wrapper if you only used CONFIG_ARCH_SHMOBILE_MULTI above. Apart from that it looks good to me. And, yes, I have tested this on my KZM9D board together with multiplatform and it works very well! Cheers, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/6] ARM: shmobile: kzm9d-reference: Use common clock framework
On Tue, Sep 24, 2013 at 1:17 PM, wrote:
> Use common clock framework version of clock
> drivers/clk/shmobile/clk-emev2.c
> instead of sh-clkfwk version
> arch/arm/mach-shmobile/clock-emev2.c
>
> kzm9d(without -reference) still uses sh-clkfwk version.
>
> Because two of that framework can not live in one kernel binary,
> there will be SoCs and Boards that can not be in one binary as
> multiplatform binary or so.
> For example, kzm9d and kzm9d-reference is now exclusive.
>
> Signed-off-by: Takashi Yoshii
> ---
> arch/arm/mach-shmobile/Kconfig | 1 +
> arch/arm/mach-shmobile/board-kzm9d-reference.c | 5 ++---
> 2 files changed, 3 insertions(+), 3 deletions(-)
Hi Yoshii-san,
Thanks for your patch. I have some comments on this portion to try to
simplify things, please see below.
> diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
> index 50bab8d..d20d4ce 100644
> --- a/arch/arm/mach-shmobile/Kconfig
> +++ b/arch/arm/mach-shmobile/Kconfig
> @@ -237,6 +237,7 @@ config MACH_KZM9D_REFERENCE
> depends on ARCH_EMEV2
> select REGULATOR_FIXED_VOLTAGE if REGULATOR
> select USE_OF
> + select COMMON_CLK
I don't think this hunk is needed. This is probably the
ARCH_SHMOBILE_MULTI case, and if so then ARCH_MULTIPLATFORM in
arch/arm/Kconfig already selects COMMON_CLK. And if it's the
ARCH_SHMOBILE case you're aiming at then I recommend you to only focus
on ARCH_SHMOBILE_MULTI instead.
> diff --git a/arch/arm/mach-shmobile/board-kzm9d-reference.c
> b/arch/arm/mach-shmobile/board-kzm9d-reference.c
> index 8f8bb2f..e0b8317 100644
> --- a/arch/arm/mach-shmobile/board-kzm9d-reference.c
> +++ b/arch/arm/mach-shmobile/board-kzm9d-reference.c
> @@ -20,15 +20,14 @@
>
> #include
> #include
> +#include
> #include
> #include
> #include
>
> static void __init kzm9d_add_standard_devices(void)
> {
> - if (!IS_ENABLED(CONFIG_COMMON_CLK))
> - emev2_clock_init();
> -
> + of_clk_init(NULL);
> of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> }
To keep on allowing build of board-kzm9d-reference.c for both
ARCH_SHMOBILE_MULTI and ARCH_SHMOBILE I recommend you to adjust your
code into something liket this instead:
@@ -18,6 +18,7 @@
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*/
+#include
#include
#include
#include
@@ -26,9 +27,11 @@
static void __init kzm9d_add_standard_devices(void)
{
- if (!IS_ENABLED(CONFIG_COMMON_CLK))
- emev2_clock_init();
-
+#ifdef CONFIG_COMMON_CLK
+ of_clk_init(NULL);
+#else
+ emev2_clock_init();
+#endif
of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
}
Cheers,
/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/6] sh: clkfwk: Select sh-/common- clkfwk alternatively
On Tue, Sep 24, 2013 at 1:12 PM, wrote: > Make sh clock framework core depend on HAVE_MACH_CLKDEV, and > set it > - y on sh for backward compatibility > - !CONFIG_COMMON_CLK on sh-mobile > This is a preparation for migration to common clock framework > from sh clock framework on sh-mobile. > > Signed-off-by: Takashi Yoshii > --- > arch/arm/Kconfig| 2 +- > arch/sh/Kconfig | 1 + > drivers/sh/clk/Makefile | 3 +-- > 3 files changed, 3 insertions(+), 3 deletions(-) Hi Yoshii-san, Thanks for your patch. I'm sure there is a reason behind this, but I'm trying to understand why you need this modification. It looks to me like you're trying to enable COMMON_CLK on ARCH_SHMOBILE but in my mind it is enough to only enable COMMON_CLK in the case of ARCH_SHMOBILE_MULTI. During my test using ARCH_SHMOBILE_MULTI on KZM9D I omitted this patch and only used patch 1, 2, 4, 5, and a modified 6 from your series. This worked just fine, but I may be missing something. Let me know if you really want to keep this patch or not. If it's not needed for MULTIPLATFORM then I suggest that we just drop it. Cheers, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH/RFC] arm: arch_timer: Do not set C3STOP in case CPU_IDLE=n
From: Magnus Damm
Modify the ARM architected timer driver to not set C3STOP
in case CPU_IDLE is disabled. This is a short term fix that
allows use of high resolution timers even though no additional
clock event is registered.
Not-really-Signed-off-by: Magnus Damm
---
If someone cares about this case then perhaps it should be
moved up to the clock event main code. The same issue should
in theory trigger on all architectures, perhaps x86 people
hunting for low latency may try to disable CPU_IDLE?
I propose carrying this patch locally to enable high resolution
timers until CPU_IDLE and an additional clock event is supported.
Observed on r8a73a4 and APE6EVM.
drivers/clocksource/arm_arch_timer.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
--- 0001/drivers/clocksource/arm_arch_timer.c
+++ work/drivers/clocksource/arm_arch_timer.c 2013-06-17 09:03:44.0
+0900
@@ -125,7 +125,23 @@ static int arch_timer_set_next_event_phy
static int __cpuinit arch_timer_setup(struct clock_event_device *clk)
{
- clk->features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP;
+ clk->features = CLOCK_EVT_FEAT_ONESHOT;
+#ifdef CONFIG_CPU_IDLE
+ /* By not setting the C3STOP flag it is possible to let the
+* ARM architected timer to be the only clock event installed
+* on the system and have working high resolution timers.
+*
+* If the C3STOP flag is set unconditionally then the kernel
+* will always prevent using the high resoultion timer feature
+* unless an additional clock event is registered.
+*
+* In the case where CPU_IDLE is enabled then there is a chance
+* that deeper sleep states will be handled by software, but
+* if CPU_IDLE is disabled then deep sleep states cannot be
+* entered and the feature flagged by C3STOP is not needed.
+*/
+ clk->features |= CLOCK_EVT_FEAT_C3STOP;
+#endif
clk->name = "arch_sys_timer";
clk->rating = 450;
if (arch_timer_use_virtual) {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH/RFC] arm: arch_timer: Do not set C3STOP in case CPU_IDLE=n
Hi Simon, On Mon, Jun 17, 2013 at 11:13 AM, Simon Horman wrote: > On Mon, Jun 17, 2013 at 09:20:56AM +0900, Magnus Damm wrote: >> From: Magnus Damm >> >> Modify the ARM architected timer driver to not set C3STOP >> in case CPU_IDLE is disabled. This is a short term fix that >> allows use of high resolution timers even though no additional >> clock event is registered. >> >> Not-really-Signed-off-by: Magnus Damm >> --- >> >> If someone cares about this case then perhaps it should be >> moved up to the clock event main code. The same issue should >> in theory trigger on all architectures, perhaps x86 people >> hunting for low latency may try to disable CPU_IDLE? >> >> I propose carrying this patch locally to enable high resolution >> timers until CPU_IDLE and an additional clock event is supported. >> >> Observed on r8a73a4 and APE6EVM. > > Hi Magnus, > > Is this patch intended to be picked up by me for the LTSI-3.4.25 based > backports that live in my renesas-backports tree? Yes, correct. The patch was mainly written to satisfy a feature request for your backports, but I noticed that the same issue exists in upstream as well. Ideally I'd like to use the same code for the backport and upstream, but I am not sure if anyone in upstream really cares. The more long term solution is obviously to install a second clock event, perhaps that's good enough. > If so, could you clearly state this (below the '---' is fine) and > include a proper Sob line to indicate that it is fit to be merged > even if that merge is not into mainline. Sure, but I'd like to hear opinions from other people before resending. I will follow your recommendation in next version. Thanks, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] clocksource: sh_cmt: 32-bit control register support
From: Magnus Damm
Add support for CMT hardware with 32-bit control and counter
registers, as found on r8a73a4 and r8a7790. To use the CMT
with 32-bit hardware a second I/O memory resource needs to
point out the CMSTR register and it needs to be 32 bit wide.
Signed-off-by: Magnus Damm
---
Tested on r8a73a4 used on APE6EVM.
drivers/clocksource/sh_cmt.c | 50 ++
1 file changed, 36 insertions(+), 14 deletions(-)
--- 0001/drivers/clocksource/sh_cmt.c
+++ work/drivers/clocksource/sh_cmt.c 2013-06-17 13:47:34.0 +0900
@@ -37,6 +37,7 @@
struct sh_cmt_priv {
void __iomem *mapbase;
+ void __iomem *mapbase_str;
struct clk *clk;
unsigned long width; /* 16 or 32 bit version of hardware block */
unsigned long overflow_bit;
@@ -79,6 +80,12 @@ struct sh_cmt_priv {
* CMCSR 0xffca0060 16-bit
* CMCNT 0xffca0064 32-bit
* CMCOR 0xffca0068 32-bit
+ *
+ * "32-bit counter and 32-bit control" as found on r8a73a4 and r8a7790:
+ * CMSTR 0xffca0500 32-bit
+ * CMCSR 0xffca0510 32-bit
+ * CMCNT 0xffca0514 32-bit
+ * CMCOR 0xffca0518 32-bit
*/
static unsigned long sh_cmt_read16(void __iomem *base, unsigned long offs)
@@ -109,9 +116,7 @@ static void sh_cmt_write32(void __iomem
static inline unsigned long sh_cmt_read_cmstr(struct sh_cmt_priv *p)
{
- struct sh_timer_config *cfg = p->pdev->dev.platform_data;
-
- return p->read_control(p->mapbase - cfg->channel_offset, 0);
+ return p->read_control(p->mapbase_str, 0);
}
static inline unsigned long sh_cmt_read_cmcsr(struct sh_cmt_priv *p)
@@ -127,9 +132,7 @@ static inline unsigned long sh_cmt_read_
static inline void sh_cmt_write_cmstr(struct sh_cmt_priv *p,
unsigned long value)
{
- struct sh_timer_config *cfg = p->pdev->dev.platform_data;
-
- p->write_control(p->mapbase - cfg->channel_offset, 0, value);
+ p->write_control(p->mapbase_str, 0, value);
}
static inline void sh_cmt_write_cmcsr(struct sh_cmt_priv *p,
@@ -676,7 +679,7 @@ static int sh_cmt_register(struct sh_cmt
static int sh_cmt_setup(struct sh_cmt_priv *p, struct platform_device *pdev)
{
struct sh_timer_config *cfg = pdev->dev.platform_data;
- struct resource *res;
+ struct resource *res, *res2;
int irq, ret;
ret = -ENXIO;
@@ -694,6 +697,9 @@ static int sh_cmt_setup(struct sh_cmt_pr
goto err0;
}
+ /* optional resource for the shared timer start/stop register */
+ res2 = platform_get_resource(p->pdev, IORESOURCE_MEM, 1);
+
irq = platform_get_irq(p->pdev, 0);
if (irq < 0) {
dev_err(&p->pdev->dev, "failed to get irq\n");
@@ -707,6 +713,15 @@ static int sh_cmt_setup(struct sh_cmt_pr
goto err0;
}
+ /* map second resource for CMSTR */
+ p->mapbase_str = ioremap_nocache(res2 ? res2->start :
+res->start - cfg->channel_offset,
+res2 ? resource_size(res2) : 2);
+ if (p->mapbase_str == NULL) {
+ dev_err(&p->pdev->dev, "failed to remap I/O second memory\n");
+ goto err1;
+ }
+
/* request irq using setup_irq() (too early for request_irq()) */
p->irqaction.name = dev_name(&p->pdev->dev);
p->irqaction.handler = sh_cmt_interrupt;
@@ -719,11 +734,17 @@ static int sh_cmt_setup(struct sh_cmt_pr
if (IS_ERR(p->clk)) {
dev_err(&p->pdev->dev, "cannot get clock\n");
ret = PTR_ERR(p->clk);
- goto err1;
+ goto err2;
}
- p->read_control = sh_cmt_read16;
- p->write_control = sh_cmt_write16;
+ if (res2 && (resource_size(res2) == 4)) {
+ /* assume both CMSTR and CMCSR to be 32-bit */
+ p->read_control = sh_cmt_read32;
+ p->write_control = sh_cmt_write32;
+ } else {
+ p->read_control = sh_cmt_read16;
+ p->write_control = sh_cmt_write16;
+ }
if (resource_size(res) == 6) {
p->width = 16;
@@ -752,22 +773,23 @@ static int sh_cmt_setup(struct sh_cmt_pr
cfg->clocksource_rating);
if (ret) {
dev_err(&p->pdev->dev, "registration failed\n");
- goto err2;
+ goto err3;
}
p->cs_enabled = false;
ret = setup_irq(irq, &p->irqaction);
if (ret) {
dev_err(&p->pdev->dev, "failed to request irq %d\n", irq);
- goto err2;
+ goto err3;
}
platform_set_drvdata(pdev, p);
Re: [PATCH] clocksource: sh_cmt: 32-bit control register support
Hi Laurent, On Tue, Jun 18, 2013 at 3:37 AM, Laurent Pinchart wrote: > Hi Magnus, > > Thanks for the patch. > > On Monday 17 June 2013 15:40:52 Magnus Damm wrote: >> From: Magnus Damm >> >> Add support for CMT hardware with 32-bit control and counter >> registers, as found on r8a73a4 and r8a7790. To use the CMT >> with 32-bit hardware a second I/O memory resource needs to >> point out the CMSTR register and it needs to be 32 bit wide. > > Is a memory second resource required ? Can't we use a single resource that > will contain all the registers ? The CMT hardware block comes with a shared timer start stop register that historically has been left out of the resource. The location of this register has so far been pointed out by the "channel offset" platform data member, together with information about which bit that happens to be assigned to the timer channel. This start stop register has happened to be kept in the same page of I/O memory as the main timer channel resource, so at this point we're sort of "lucky" that a single ioremap() has covered all cases. With this patch it becomes optional to instead of platform data use a second resource to point out the timer start/stop register. While we do that we can also use the size of that resource to determine the I/O access width, which happens to be something that is needed to enable the driver on certain SoCs. > Time to switch to devm_* managed functions ? :-) Yes, indeed. That among other things, like converting the driver to in a more optimal way support clock source only or clock event only configurations. Also, some more modern CMT hardware versions have extended registers with 48-bit counters, and we can also often use more high frequency clocks to improve timer resolution. As you can tell, in general there are many things that can be improved with this driver. I thought a first shot could be to make it actually work on more recent CMT hardware with 32-bit only registers. So that's what this patch does! Cheers, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH/RFC] clockevents: Ignore C3STOP when CPUIdle is disabled
From: Magnus Damm
Introduce the function tick_device_may_c3stop() that
ignores the C3STOP flag in case CPUIdle is disabled.
The C3STOP flag tells the system that a clock event
device may be stopped during deep sleep, but if this
will happen or not depends on things like if CPUIdle
is enabled and if a CPUIdle driver is available.
This patch assumes that if CPUIdle is disabled then
the sleep mode triggering C3STOP will never be entered.
So by ignoring C3STOP when CPUIdle is disabled then it
becomes possible to use high resolution timers with only
per-cpu local timers - regardless if they have the
C3STOP flag set or not.
Observed on the r8a73a4 SoC that at this point only uses
ARM architected timers for clock event and clock sources.
Without this patch high resolution timers are run time
disabled on the r8a73a4 SoC - this regardless of CPUIdle
is disabled or not.
The less short term fix is to add support for more timers
on the r8a73a4 SoC, but until CPUIdle support is enabled
it must be possible to use high resoultion timers without
additional timers.
I'd like to hear some feedback and also test this on more
systems before merging the code, see the non-SOB below.
Not-Yet-Signed-off-by: Magnus Damm
---
An earlier ARM arch timer specific version of this patch was
posted yesterday as:
"[PATCH/RFC] arm: arch_timer: Do not set C3STOP in case CPU_IDLE=n"
Many thanks to Mark Rutland for his kind feedback.
kernel/time/tick-broadcast.c |8
kernel/time/tick-common.c|2 +-
kernel/time/tick-internal.h | 11 +++
3 files changed, 16 insertions(+), 5 deletions(-)
--- 0001/kernel/time/tick-broadcast.c
+++ work/kernel/time/tick-broadcast.c 2013-06-18 15:36:21.0 +0900
@@ -71,7 +71,7 @@ int tick_check_broadcast_device(struct c
if ((dev->features & CLOCK_EVT_FEAT_DUMMY) ||
(tick_broadcast_device.evtdev &&
tick_broadcast_device.evtdev->rating >= dev->rating) ||
-(dev->features & CLOCK_EVT_FEAT_C3STOP))
+tick_device_may_c3stop(dev))
return 0;
clockevents_exchange_device(tick_broadcast_device.evtdev, dev);
@@ -146,7 +146,7 @@ int tick_device_uses_broadcast(struct cl
* feature and the cpu is marked in the broadcast mask
* then clear the broadcast bit.
*/
- if (!(dev->features & CLOCK_EVT_FEAT_C3STOP)) {
+ if (!tick_device_may_c3stop(dev)) {
int cpu = smp_processor_id();
cpumask_clear_cpu(cpu, tick_broadcast_mask);
tick_broadcast_clear_oneshot(cpu);
@@ -270,7 +270,7 @@ static void tick_do_broadcast_on_off(uns
/*
* Is the device not affected by the powerstate ?
*/
- if (!dev || !(dev->features & CLOCK_EVT_FEAT_C3STOP))
+ if (!dev || !tick_device_may_c3stop(dev))
goto out;
if (!tick_device_is_functional(dev))
@@ -568,7 +568,7 @@ void tick_broadcast_oneshot_control(unsi
td = &per_cpu(tick_cpu_device, cpu);
dev = td->evtdev;
- if (!(dev->features & CLOCK_EVT_FEAT_C3STOP))
+ if (!tick_device_may_c3stop(dev))
return;
bc = tick_broadcast_device.evtdev;
--- 0001/kernel/time/tick-common.c
+++ work/kernel/time/tick-common.c 2013-06-18 15:36:29.0 +0900
@@ -52,7 +52,7 @@ int tick_is_oneshot_available(void)
if (!dev || !(dev->features & CLOCK_EVT_FEAT_ONESHOT))
return 0;
- if (!(dev->features & CLOCK_EVT_FEAT_C3STOP))
+ if (!tick_device_may_c3stop(dev))
return 1;
return tick_broadcast_oneshot_available();
}
--- 0001/kernel/time/tick-internal.h
+++ work/kernel/time/tick-internal.h2013-06-18 15:40:10.0 +0900
@@ -141,6 +141,17 @@ static inline int tick_device_is_functio
return !(dev->features & CLOCK_EVT_FEAT_DUMMY);
}
+/*
+ * Check, if the device has C3STOP behavior and CPU Idle is enabled
+ */
+static inline bool tick_device_may_c3stop(struct clock_event_device *dev)
+{
+ /* The C3 sleep mode can only trigger when CPU Idle is enabled,
+* so if CPU Idle is disabled then the C3STOP flag can be ignored */
+ return (IS_ENABLED(CONFIG_CPU_IDLE) &&
+ (dev->features & CLOCK_EVT_FEAT_C3STOP));
+}
+
#endif
extern void do_timer(unsigned long ticks);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH/RFC] arm: arch_timer: Do not set C3STOP in case CPU_IDLE=n
Hi Mark, On Mon, Jun 17, 2013 at 11:53 PM, Mark Rutland wrote: > On Mon, Jun 17, 2013 at 01:20:56AM +0100, Magnus Damm wrote: >> From: Magnus Damm >> >> Modify the ARM architected timer driver to not set C3STOP >> in case CPU_IDLE is disabled. This is a short term fix that >> allows use of high resolution timers even though no additional >> clock event is registered. >> >> Not-really-Signed-off-by: Magnus Damm >> --- >> >> If someone cares about this case then perhaps it should be >> moved up to the clock event main code. The same issue should >> in theory trigger on all architectures, perhaps x86 people >> hunting for low latency may try to disable CPU_IDLE? > > I think that changing tick_is_oneshot_capable and friends to only worry about > C3STOP when CPU_IDLE is enabled would be a nicer solution. That way you enable > all clock_event_devices with C3STOP to function as high resolution timers when > CPU_IDLE's selected. Presenting the hardware differently depending on CPU_IDLE > feels wrong. I agree that doing this in the clock event driver is a bit odd. So because of that I just posted this patch: [PATCH/RFC] clockevents: Ignore C3STOP when CPUIdle is disabled > Having some other clock_event_device would be a nicer solution still. No doubt that another clock event device helps, but mainly together with CPU Idle IMO. Thanks for your comments! / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH/RFC] clockevents: Ignore C3STOP when CPUIdle is disabled
On Tue, Jun 18, 2013 at 4:32 PM, Daniel Lezcano wrote: > On 06/18/2013 09:17 AM, Magnus Damm wrote: >> From: Magnus Damm >> >> Introduce the function tick_device_may_c3stop() that >> ignores the C3STOP flag in case CPUIdle is disabled. >> >> The C3STOP flag tells the system that a clock event >> device may be stopped during deep sleep, but if this >> will happen or not depends on things like if CPUIdle >> is enabled and if a CPUIdle driver is available. >> >> This patch assumes that if CPUIdle is disabled then >> the sleep mode triggering C3STOP will never be entered. >> So by ignoring C3STOP when CPUIdle is disabled then it >> becomes possible to use high resolution timers with only >> per-cpu local timers - regardless if they have the >> C3STOP flag set or not. >> >> Observed on the r8a73a4 SoC that at this point only uses >> ARM architected timers for clock event and clock sources. >> >> Without this patch high resolution timers are run time >> disabled on the r8a73a4 SoC - this regardless of CPUIdle >> is disabled or not. >> >> The less short term fix is to add support for more timers >> on the r8a73a4 SoC, but until CPUIdle support is enabled >> it must be possible to use high resoultion timers without >> additional timers. >> >> I'd like to hear some feedback and also test this on more >> systems before merging the code, see the non-SOB below. > > Do we need a broadcast timer when cpuidle is not compiled in the kernel ? Yes, if there is no per-cpu timer available. It depends on what the SMP support code for a particular SoC or architecture happen to enable. / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH/RFC] clockevents: Ignore C3STOP when CPUIdle is disabled
Hi Daniel, On Tue, Jun 18, 2013 at 5:24 PM, Daniel Lezcano wrote: > On 06/18/2013 09:39 AM, Magnus Damm wrote: >> On Tue, Jun 18, 2013 at 4:32 PM, Daniel Lezcano >> wrote: >>> On 06/18/2013 09:17 AM, Magnus Damm wrote: >>>> From: Magnus Damm >>>> >>>> Introduce the function tick_device_may_c3stop() that >>>> ignores the C3STOP flag in case CPUIdle is disabled. >>>> >>>> The C3STOP flag tells the system that a clock event >>>> device may be stopped during deep sleep, but if this >>>> will happen or not depends on things like if CPUIdle >>>> is enabled and if a CPUIdle driver is available. >>>> >>>> This patch assumes that if CPUIdle is disabled then >>>> the sleep mode triggering C3STOP will never be entered. >>>> So by ignoring C3STOP when CPUIdle is disabled then it >>>> becomes possible to use high resolution timers with only >>>> per-cpu local timers - regardless if they have the >>>> C3STOP flag set or not. >>>> >>>> Observed on the r8a73a4 SoC that at this point only uses >>>> ARM architected timers for clock event and clock sources. >>>> >>>> Without this patch high resolution timers are run time >>>> disabled on the r8a73a4 SoC - this regardless of CPUIdle >>>> is disabled or not. >>>> >>>> The less short term fix is to add support for more timers >>>> on the r8a73a4 SoC, but until CPUIdle support is enabled >>>> it must be possible to use high resoultion timers without >>>> additional timers. >>>> >>>> I'd like to hear some feedback and also test this on more >>>> systems before merging the code, see the non-SOB below. >>> >>> Do we need a broadcast timer when cpuidle is not compiled in the kernel ? >> >> Yes, if there is no per-cpu timer available. It depends on what the >> SMP support code for a particular SoC or architecture happen to >> enable. > > Ok thanks for the information. No problem. Thanks for your comments! > There is here a multiple occurrence of the information "the timer will > stop when power is saved": CLOCK_EVT_FEAT_C3STOP and > CPUIDLE_FLAG_TIMER_STOP, so I am wondering if some code simplification > couldn't be done before your patch. I'm sure it's possible to rearrange things in many ways, and the area that you point out indeed seems to have some overlap. Somehow describing which timers that stop during what CPUIdle sleep state would be nice to have. Also, today clock event drivers simply state C3STOP but there may be shallow sleep modes where the timer doesn't have to stop. It all seems a bit coarse grained to me as-is. > The function: > > tick_broadcast_oneshot_control is called from clockevents_notify. This > one is called from the cpuidle framework or the back-end cpuidle driver. > The caller knows the timer will be stop and this is why it is switching > to the broadcast mode. But we have a sanity check in > tick_broadcast_oneshot_control function: > > if (!(dev->features & CLOCK_EVT_FEAT_C3STOP)) > return; > > In other words, CPUIDLE_FLAG_TIMER_STOP will tell the framework to call > clockevents_notify and the tick broadcast code will re-check the device > will effectively go down. IMHO, we can get rid of this check. > > The same happens for the tick_do_broadcast_on_off function. > > That reduces the number of C3STOP usage. That may very well be the case. Care to hack up a patch? =) The goal with this patch is simply to make it possible to use high resolution timers if CPUIdle is disabled. Right now the ARM architected timer is sort of optimized for power, so it sets the C3STOP flag to say that on some SoCs during some sleep modes these timers may stop. My point is that this flag doesn't matter as long as CPUIdle is disabled. Thanks, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH/RFC] clockevents: Ignore C3STOP when CPUIdle is disabled
Hi Daniel,
On Tue, Jun 18, 2013 at 5:30 PM, Daniel Lezcano
wrote:
> On 06/18/2013 09:17 AM, Magnus Damm wrote:
>> From: Magnus Damm
>>
>> Introduce the function tick_device_may_c3stop() that
>> ignores the C3STOP flag in case CPUIdle is disabled.
>>
>> The C3STOP flag tells the system that a clock event
>> device may be stopped during deep sleep, but if this
>> will happen or not depends on things like if CPUIdle
>> is enabled and if a CPUIdle driver is available.
>>
>> This patch assumes that if CPUIdle is disabled then
>> the sleep mode triggering C3STOP will never be entered.
>> So by ignoring C3STOP when CPUIdle is disabled then it
>> becomes possible to use high resolution timers with only
>> per-cpu local timers - regardless if they have the
>> C3STOP flag set or not.
>>
>> Observed on the r8a73a4 SoC that at this point only uses
>> ARM architected timers for clock event and clock sources.
>>
>> Without this patch high resolution timers are run time
>> disabled on the r8a73a4 SoC - this regardless of CPUIdle
>> is disabled or not.
>>
>> The less short term fix is to add support for more timers
>> on the r8a73a4 SoC, but until CPUIdle support is enabled
>> it must be possible to use high resoultion timers without
>> additional timers.
>>
>> I'd like to hear some feedback and also test this on more
>> systems before merging the code, see the non-SOB below.
>>
>> Not-Yet-Signed-off-by: Magnus Damm
>> ---
>>
>> An earlier ARM arch timer specific version of this patch was
>> posted yesterday as:
>> "[PATCH/RFC] arm: arch_timer: Do not set C3STOP in case CPU_IDLE=n"
>>
>> Many thanks to Mark Rutland for his kind feedback.
>>
>> kernel/time/tick-broadcast.c |8
>> kernel/time/tick-common.c|2 +-
>> kernel/time/tick-internal.h | 11 +++
>> 3 files changed, 16 insertions(+), 5 deletions(-)
>>
>> --- 0001/kernel/time/tick-broadcast.c
>> +++ work/kernel/time/tick-broadcast.c 2013-06-18 15:36:21.0 +0900
>> @@ -71,7 +71,7 @@ int tick_check_broadcast_device(struct c
>> if ((dev->features & CLOCK_EVT_FEAT_DUMMY) ||
>> (tick_broadcast_device.evtdev &&
>>tick_broadcast_device.evtdev->rating >= dev->rating) ||
>> - (dev->features & CLOCK_EVT_FEAT_C3STOP))
>> + tick_device_may_c3stop(dev))
>> return 0;
>>
>> clockevents_exchange_device(tick_broadcast_device.evtdev, dev);
>> @@ -146,7 +146,7 @@ int tick_device_uses_broadcast(struct cl
>>* feature and the cpu is marked in the broadcast mask
>>* then clear the broadcast bit.
>>*/
>> - if (!(dev->features & CLOCK_EVT_FEAT_C3STOP)) {
>> + if (!tick_device_may_c3stop(dev)) {
>> int cpu = smp_processor_id();
>> cpumask_clear_cpu(cpu, tick_broadcast_mask);
>> tick_broadcast_clear_oneshot(cpu);
>> @@ -270,7 +270,7 @@ static void tick_do_broadcast_on_off(uns
>> /*
>>* Is the device not affected by the powerstate ?
>>*/
>> - if (!dev || !(dev->features & CLOCK_EVT_FEAT_C3STOP))
>> + if (!dev || !tick_device_may_c3stop(dev))
>> goto out;
>>
>> if (!tick_device_is_functional(dev))
>> @@ -568,7 +568,7 @@ void tick_broadcast_oneshot_control(unsi
>> td = &per_cpu(tick_cpu_device, cpu);
>> dev = td->evtdev;
>>
>> - if (!(dev->features & CLOCK_EVT_FEAT_C3STOP))
>> + if (!tick_device_may_c3stop(dev))
>> return;
>>
>> bc = tick_broadcast_device.evtdev;
>> --- 0001/kernel/time/tick-common.c
>> +++ work/kernel/time/tick-common.c2013-06-18 15:36:29.0 +0900
>> @@ -52,7 +52,7 @@ int tick_is_oneshot_available(void)
>>
>> if (!dev || !(dev->features & CLOCK_EVT_FEAT_ONESHOT))
>> return 0;
>> - if (!(dev->features & CLOCK_EVT_FEAT_C3STOP))
>> + if (!tick_device_may_c3stop(dev))
>> return 1;
>> return tick_broadcast_oneshot_available();
>> }
>> --- 0001/kernel/time/tick-internal.h
>> +++ work/kernel/time/tick-internal.h 2013-06-18 15:40:10.0 +0900
>> @@ -141,6 +141,17 @@ static inline int tick_device_is_functio
>> return !(dev->features & CLOCK_EVT_FEAT_DUMMY);
>> }
>>
>> +/*
&
Re: [PATCH] clocksource: sh_cmt: 32-bit control register support
Hi Laurent, On Tue, Jun 18, 2013 at 7:35 PM, Laurent Pinchart wrote: > Hi Magnus, > > On Tuesday 18 June 2013 14:39:38 Magnus Damm wrote: >> On Tue, Jun 18, 2013 at 3:37 AM, Laurent Pinchart wrote: >> > On Monday 17 June 2013 15:40:52 Magnus Damm wrote: >> >> From: Magnus Damm >> >> >> >> Add support for CMT hardware with 32-bit control and counter >> >> registers, as found on r8a73a4 and r8a7790. To use the CMT >> >> with 32-bit hardware a second I/O memory resource needs to >> >> point out the CMSTR register and it needs to be 32 bit wide. >> > >> > Is a memory second resource required ? Can't we use a single resource that >> > will contain all the registers ? >> >> The CMT hardware block comes with a shared timer start stop register >> that historically has been left out of the resource. The location of >> this register has so far been pointed out by the "channel offset" >> platform data member, together with information about which bit that >> happens to be assigned to the timer channel. This start stop register >> has happened to be kept in the same page of I/O memory as the main >> timer channel resource, so at this point we're sort of "lucky" that a >> single ioremap() has covered all cases. >> >> With this patch it becomes optional to instead of platform data use a >> second resource to point out the timer start/stop register. While we >> do that we can also use the size of that resource to determine the I/O >> access width, which happens to be something that is needed to enable >> the driver on certain SoCs. > > OK, I get it now. I've had a quick look at the documentation, and I'm > wondering whether we shouldn't register a single platform device that span all > the channels contained in the CMT, instead of registering one platform device > per channel. I both agree with you and disagree because of the current state of timers in the linux kernel. I would have liked a single platform device with all channles if this would be a generic timer driver that from user space could be configured to associate channels with various subsystems like PWM, clocksource, clockevent. At this point the driver is doing clockevent and clocksource only, and no sane user wants 84 channels of equivalent hardware blocks for those two. So based on that I'd rather do it like today and let people write custom drivers for whatever applications they may use the other channels for. So if you're in hacking mode, why don't you figure out some way timers can be configured from user space? =) If so then we can use DT to describe the actual hardware and let the software policy be decided via some configuration mechanism. Cheers, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clocksource: sh_cmt: 32-bit control register support
Hi Laurent, On Tue, Jun 18, 2013 at 9:30 PM, Laurent Pinchart wrote: > Hi Magnus, > > On Tuesday 18 June 2013 20:54:47 Magnus Damm wrote: >> On Tue, Jun 18, 2013 at 7:35 PM, Laurent Pinchart wrote: >> > On Tuesday 18 June 2013 14:39:38 Magnus Damm wrote: >> >> On Tue, Jun 18, 2013 at 3:37 AM, Laurent Pinchart wrote: >> >> > On Monday 17 June 2013 15:40:52 Magnus Damm wrote: >> >> >> From: Magnus Damm >> >> >> >> >> >> Add support for CMT hardware with 32-bit control and counter >> >> >> registers, as found on r8a73a4 and r8a7790. To use the CMT >> >> >> with 32-bit hardware a second I/O memory resource needs to >> >> >> point out the CMSTR register and it needs to be 32 bit wide. >> >> > >> >> > Is a memory second resource required ? Can't we use a single resource >> >> > that will contain all the registers ? >> >> >> >> The CMT hardware block comes with a shared timer start stop register >> >> that historically has been left out of the resource. The location of >> >> this register has so far been pointed out by the "channel offset" >> >> platform data member, together with information about which bit that >> >> happens to be assigned to the timer channel. This start stop register >> >> has happened to be kept in the same page of I/O memory as the main >> >> timer channel resource, so at this point we're sort of "lucky" that a >> >> single ioremap() has covered all cases. >> >> >> >> With this patch it becomes optional to instead of platform data use a >> >> second resource to point out the timer start/stop register. While we >> >> do that we can also use the size of that resource to determine the I/O >> >> access width, which happens to be something that is needed to enable >> >> the driver on certain SoCs. >> > >> > OK, I get it now. I've had a quick look at the documentation, and I'm >> > wondering whether we shouldn't register a single platform device that span >> > all the channels contained in the CMT, instead of registering one >> > platform device per channel. >> >> I both agree with you and disagree because of the current state of timers in >> the linux kernel. I would have liked a single platform device with all >> channles if this would be a generic timer driver that from user space could >> be configured to associate channels with various subsystems like PWM, >> clocksource, clockevent. >> >> At this point the driver is doing clockevent and clocksource only, and no >> sane user wants 84 channels of equivalent hardware blocks for those two. > > Of course, but we could always select which channels to register clockevents > and clocksources for in platform data. That won't fix the overall problem, but > it's one step forward. But that's pretty much what we're doing, but only listing timer channels that will be used. Of course, moving around things can be done but I can't see why we want to do that if we have no selection of drivers for the actual timer channels. Also, each timer channel may have it's own unique set of possible parent clocks. That's something we want to tie in to DT together with CCF. Solving those things together makes sense IMO. >> So based on that I'd rather do it like today and let people write custom >> drivers for whatever applications they may use the other channels for. >> >> So if you're in hacking mode, why don't you figure out some way timers can >> be configured from user space? =) > > I don't have *that* much free time at the moment I'm afraid, and I'm sure you > know why :-) Yes I do, and that's why I asked. =) >> If so then we can use DT to describe the actual hardware and let the >> software policy be decided via some configuration mechanism. > > Don't we also need timers during early boot, when userspace isn't available > yet ? It depends on the rest of the system. It is possible to boot to user space without a timer, but I don't recommend it. =) Cheers, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] clocksource: sh_cmt: Use devm_* managed helpers
On Tue, Jun 18, 2013 at 9:36 PM, Laurent Pinchart wrote: > This simplifies the main error path by getting rid of it. > > Signed-off-by: Laurent Pinchart Looking good, thanks! Acked-by: Magnus Damm -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] arm: Prevent memory aliasing on non-LPAE kernels
Hello Stepan, On Fri, May 31, 2013 at 6:45 AM, Stepan Moskovchenko wrote: > Some LPAE-capable systems may use a Device Tree containing > memory nodes that describe memory extending beyond the 4GB > physical address boundary. Ignore or truncate these memory > nodes on kernels that have not been built with LPAE > support, to prevent the extended physical addresses from > being truncated and aliasing with physical addresses below > the 4GB boundary. > > Signed-off-by: Stepan Moskovchenko > --- > arch/arm/kernel/devtree.c | 12 > 1 files changed, 12 insertions(+), 0 deletions(-) Thanks for your efforts on fixing this issue. Before I was aware of this patch I wrote a different implementation to solve most likely the same issue, please see the following patches for more information. Thanks to Arnd for pointing me in the right direction. [PATCH 00/03] ARM: 64-bit memory fixes, APE6EVM second memory bank [PATCH 01/03] ARM: Let arm_add_memory() always use 64-bit arguments [PATCH 02/03] ARM: Handle 64-bit memory in case of 32-bit phys_addr_t [PATCH 03/03] ARM: shmobile: Add second memory bank to DTS for APE6EVM Regarding this patch, I have now tested it on my APE6EVM board together with this patch: [PATCH 03/03] ARM: shmobile: Add second memory bank to DTS for APE6EVM Without your patch the situation is as follows: HIGHMEM=n, LPAE=n - OK (busted, second bank ignored with message [1]) HIGHMEM=y, LPAE=n - NG (busted, board hangs on boot) HIGHMEM=n, LPAE=y - OK HIGHMEM=y, LPAE=y - OK [1] Ignoring RAM at -3fff (vmalloc region overlap). With your patch applied I get the following: HIGHMEM=n, LPAE=n - OK (with message [2]) HIGHMEM=y, LPAE=n - OK (with message [2]) HIGHMEM=n, LPAE=y - OK HIGHMEM=y, LPAE=y - OK [2] Ignoring memory at 0x2 due to lack of LPAE support So your patch unbreaks the second memory on my board perfectly well, thank you! Regarding implementation details, I wonder if we only need to cover the DT memory banks by performing the check inside early_init_dt_add_memory_arch()? To me the root cause of this issue seems to be how phys_addr_t is configured when LPAE=n. It is understandable that the kernel cannot handle 64-bit addresses when phys_addr_t is 32-bit, but I believe we need some sane way to omit those memory banks. Your patch handles the non-LPAE case before phys_addr_t is involved which seems to work well. Your approach is much better compared to as-is today with potentially wrapping phys_addr_t parameters to arm_add_memory(). The only question in my mind is about the location for this kind of test, shall it be done in early_init_dt_add_memory_arch() or arm_add_memory()? If we care about adding some bounds checking for the kernel command line mem=xxx option then arm_add_memory() seems to be the best location from my point of view. Any ideas? Please add me to CC if you respin your patch. I will give it a go on my board. Thanks, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 3/5] ARM: mach-shmobile: kota2: Use leds-pwm + pwm-rmob
Hi Laurent,
[Added Simon to CC]
Thanks for your work on converting the TPU driver. I found one issue, see below:
On Tue, Jun 11, 2013 at 10:45 PM, Laurent Pinchart
wrote:
> Instead of using the LED-specific TPU PWM driver, switch to the generic
> TPU PWM driver with leds-pwm.
>
> Signed-off-by: Laurent Pinchart
> Signed-off-by: Simon Horman
> ---
> arch/arm/mach-shmobile/board-kota2.c | 183
> --
> arch/arm/mach-shmobile/clock-sh73a0.c | 8 +-
> 2 files changed, 114 insertions(+), 77 deletions(-)
>
[snip]
> diff --git a/arch/arm/mach-shmobile/clock-sh73a0.c
> b/arch/arm/mach-shmobile/clock-sh73a0.c
> index d9fd033..6c04348 100644
> --- a/arch/arm/mach-shmobile/clock-sh73a0.c
> +++ b/arch/arm/mach-shmobile/clock-sh73a0.c
> @@ -669,10 +669,10 @@ static struct clk_lookup lookups[] = {
> CLKDEV_DEV_ID("e6bd.mmcif", &mstp_clks[MSTP312]), /* MMCIF0 */
> CLKDEV_DEV_ID("sh_mobile_sdhi.2", &mstp_clks[MSTP311]), /* SDHI2 */
> CLKDEV_DEV_ID("ee14.sdhi", &mstp_clks[MSTP311]), /* SDHI2 */
> - CLKDEV_DEV_ID("leds-renesas-tpu.12", &mstp_clks[MSTP303]), /* TPU1 */
> - CLKDEV_DEV_ID("leds-renesas-tpu.21", &mstp_clks[MSTP302]), /* TPU2 */
> - CLKDEV_DEV_ID("leds-renesas-tpu.30", &mstp_clks[MSTP301]), /* TPU3 */
> - CLKDEV_DEV_ID("leds-renesas-tpu.41", &mstp_clks[MSTP300]), /* TPU4 */
> + CLKDEV_DEV_ID("rmob_tpu_pwm.1", &mstp_clks[MSTP303]), /* TPU1 */
> + CLKDEV_DEV_ID("rmob_tpu_pwm.2", &mstp_clks[MSTP302]), /* TPU2 */
> + CLKDEV_DEV_ID("rmob_tpu_pwm.3", &mstp_clks[MSTP301]), /* TPU3 */
> + CLKDEV_DEV_ID("rmob_tpu_pwm.4", &mstp_clks[MSTP300]), /* TPU4 */
> CLKDEV_DEV_ID("i2c-sh_mobile.3", &mstp_clks[MSTP411]), /* I2C3 */
> CLKDEV_DEV_ID("e6826000.i2c", &mstp_clks[MSTP411]), /* I2C3 */
> CLKDEV_DEV_ID("i2c-sh_mobile.4", &mstp_clks[MSTP410]), /* I2C4 */
> --
> 1.8.1.5
>
I believe you are using the old "rmob" device name in the hunk above.
Apart from that it looks fine.
I left my Kota2 board at Simon's place some time ago so I'm afraind
that I can't test. Given the complexity with multiple subsystems and
also recent name changes, perhaps testing this would make sense?
Simon, can you please test this series on Kota2?
Cheers,
/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] gpio: em: Add pinctrl support
From: Magnus Damm
Register the GPIO pin range, and request and free GPIO pins using the
pinctrl API. The pctl_name platform data member should be used by
platform devices to point out which pinctrl device to use.
Follows same style as "dc3465a gpio-rcar: Add pinctrl support",
by Laurent Pinchart, thanks to him.
Signed-off-by: Magnus Damm
---
drivers/gpio/gpio-em.c| 25 +
include/linux/platform_data/gpio-em.h |1 +
2 files changed, 26 insertions(+)
--- 0001/drivers/gpio/gpio-em.c
+++ work/drivers/gpio/gpio-em.c 2013-07-03 12:49:55.0 +0900
@@ -30,6 +30,7 @@
#include
#include
#include
+#include
#include
struct em_gio_priv {
@@ -216,6 +217,21 @@ static int em_gio_to_irq(struct gpio_chi
return irq_create_mapping(gpio_to_priv(chip)->irq_domain, offset);
}
+static int em_gio_request(struct gpio_chip *chip, unsigned offset)
+{
+ return pinctrl_request_gpio(chip->base + offset);
+}
+
+static void em_gio_free(struct gpio_chip *chip, unsigned offset)
+{
+ pinctrl_free_gpio(chip->base + offset);
+
+ /* Set the GPIO as an input to ensure that the next GPIO request won't
+ * drive the GPIO pin as an output.
+ */
+ em_gio_direction_input(chip, offset);
+}
+
static int em_gio_irq_domain_map(struct irq_domain *h, unsigned int virq,
irq_hw_number_t hw)
{
@@ -308,6 +324,8 @@ static int em_gio_probe(struct platform_
gpio_chip->direction_output = em_gio_direction_output;
gpio_chip->set = em_gio_set;
gpio_chip->to_irq = em_gio_to_irq;
+ gpio_chip->request = em_gio_request;
+ gpio_chip->free = em_gio_free;
gpio_chip->label = name;
gpio_chip->owner = THIS_MODULE;
gpio_chip->base = pdata->gpio_base;
@@ -351,6 +369,13 @@ static int em_gio_probe(struct platform_
dev_err(&pdev->dev, "failed to add GPIO controller\n");
goto err1;
}
+
+ if (pdata->pctl_name) {
+ ret = gpiochip_add_pin_range(gpio_chip, pdata->pctl_name, 0,
+gpio_chip->base, gpio_chip->ngpio);
+ if (ret < 0)
+ dev_warn(&pdev->dev, "failed to add pin range\n");
+ }
return 0;
err1:
--- 0001/include/linux/platform_data/gpio-em.h
+++ work/include/linux/platform_data/gpio-em.h 2013-07-03 12:45:27.0
+0900
@@ -5,6 +5,7 @@ struct gpio_em_config {
unsigned int gpio_base;
unsigned int irq_base;
unsigned int number_of_pins;
+ const char *pctl_name;
};
#endif /* __GPIO_EM_H__ */
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] gpio: Renesas R-Car GPIO driver V3
From: Magnus Damm
This patch is V3 of a GPIO driver for the R-Car series of
SoCs from Renesas. This driver is designed to be reusable
between multiple SoCs that share the same basic building block,
but so far it has only been used on R-Car H1 (r8a7779).
Each driver instance handles 32 GPIOs with individually
maskable IRQs. The driver operates on a single I/O memory
range and the 32 GPIOs are hooked up a single interrupt.
In the case of R-Car H1 either external IRQ pins or GPIOs
with interrupts can be used for on-board interupts. For
external IRQs 4 pins are supported, and in the case of GPIO
there are 202 GPIOS as 202 interrupts hooked up via 6 driver
instances and to the GIC and the Cortex-A9 Quad.
At this point this driver is interfacing as a regular
platform device driver. In the future DT support will be
submitted as an incremental feature patch.
Signed-off-by: Magnus Damm
---
Changes since V2:
- Simple merge of [PATCH 00/03] gpio: Renesas R-Car GPIO driver update
Changes since V1:
- Update based on most suggestions from review by Laurent, thanks!
drivers/gpio/Kconfig|6
drivers/gpio/Makefile |1
drivers/gpio/gpio-rcar.c| 373 +++
include/linux/platform_data/gpio-rcar.h | 25 ++
4 files changed, 405 insertions(+)
--- 0001/drivers/gpio/Kconfig
+++ work/drivers/gpio/Kconfig 2013-03-26 09:15:56.0 +0900
@@ -204,6 +204,12 @@ config GPIO_PXA
help
Say yes here to support the PXA GPIO device
+config GPIO_RCAR
+ tristate "Renesas R-Car GPIO"
+ depends on ARM
+ help
+ Say yes here to support GPIO on Renesas R-Car SoCs.
+
config GPIO_SPEAR_SPICS
bool "ST SPEAr13xx SPI Chip Select as GPIO support"
depends on PLAT_SPEAR
--- 0001/drivers/gpio/Makefile
+++ work/drivers/gpio/Makefile 2013-03-26 09:15:56.0 +0900
@@ -57,6 +57,7 @@ obj-$(CONFIG_GPIO_PL061) += gpio-pl061.o
obj-$(CONFIG_GPIO_PXA) += gpio-pxa.o
obj-$(CONFIG_GPIO_RC5T583) += gpio-rc5t583.o
obj-$(CONFIG_GPIO_RDC321X) += gpio-rdc321x.o
+obj-$(CONFIG_GPIO_RCAR)+= gpio-rcar.o
obj-$(CONFIG_PLAT_SAMSUNG) += gpio-samsung.o
obj-$(CONFIG_ARCH_SA1100) += gpio-sa1100.o
obj-$(CONFIG_GPIO_SCH) += gpio-sch.o
--- /dev/null
+++ work/drivers/gpio/gpio-rcar.c 2013-03-26 09:16:09.0 +0900
@@ -0,0 +1,373 @@
+/*
+ * Renesas R-Car GPIO Support
+ *
+ * Copyright (C) 2013 Magnus Damm
+ *
+ * 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
+ *
+ * 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
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct gpio_rcar_priv {
+ void __iomem *base;
+ spinlock_t lock;
+ struct gpio_rcar_config config;
+ struct platform_device *pdev;
+ struct gpio_chip gpio_chip;
+ struct irq_chip irq_chip;
+ struct irq_domain *irq_domain;
+};
+
+#define IOINTSEL 0x00
+#define INOUTSEL 0x04
+#define OUTDT 0x08
+#define INDT 0x0c
+#define INTDT 0x10
+#define INTCLR 0x14
+#define INTMSK 0x18
+#define MSKCLR 0x1c
+#define POSNEG 0x20
+#define EDGLEVEL 0x24
+#define FILONOFF 0x28
+
+static inline u32 gpio_rcar_read(struct gpio_rcar_priv *p, int offs)
+{
+ return ioread32(p->base + offs);
+}
+
+static inline void gpio_rcar_write(struct gpio_rcar_priv *p, int offs,
+ u32 value)
+{
+ iowrite32(value, p->base + offs);
+}
+
+static void gpio_rcar_modify_bit(struct gpio_rcar_priv *p, int offs,
+int bit, bool value)
+{
+ u32 tmp = gpio_rcar_read(p, offs);
+
+ if (value)
+ tmp |= BIT(bit);
+ else
+ tmp &= ~BIT(bit);
+
+ gpio_rcar_write(p, offs, tmp);
+}
+
+static void gpio_rcar_irq_disable(struct irq_data *d)
+{
+ struct gpio_rcar_priv *p = irq_data_get_irq_chip_data(d);
+
+ gpio_rcar_write(p, INTMSK, ~BIT(irqd_to_hwirq(d)));
+}
+
+static void gpio_rcar_irq_enable(struct irq_data *d)
+{
+ struct gpio_rcar_priv *p = irq_data_get_irq_chip_data(d);
+
+ gpio_rcar_write(p, MSKCLR, BIT(irqd_to_hwirq(d)));
+}
+
+static void gpio_rcar_config_interrupt_input_mode(struct gpio_rcar_priv *p,
+ unsigned int hwirq,
+ bool active_high_rising_edge,
+ bool level_trigger)
+{
+ unsigned long flags;
+
+
Re: [PATCH] gpio: Renesas R-Car GPIO driver V3
Hi Laurent, On Tue, Mar 26, 2013 at 10:03 PM, Laurent Pinchart wrote: > Hi Magnus, > > Thanks for the patch. > > On Tuesday 26 March 2013 09:23:01 Magnus Damm wrote: >> From: Magnus Damm >> >> This patch is V3 of a GPIO driver for the R-Car series of >> SoCs from Renesas. This driver is designed to be reusable >> between multiple SoCs that share the same basic building block, >> but so far it has only been used on R-Car H1 (r8a7779). >> Signed-off-by: Magnus Damm > > Acked-by: Laurent Pinchart > > I've taken the patch in my tree. That's great, thanks! / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ARM: shmobile: emev2: add PMU information to emev2.dtsi
Hi Simon, On Wed, Jul 31, 2013 at 4:05 PM, Simon Horman wrote: > On Wed, Jul 31, 2013 at 03:39:04PM +0900, Simon Horman wrote: >> On Wed, Jul 24, 2013 at 12:42:40PM +0900, Magnus Damm wrote: >> > From: Magnus Damm >> > >> > Add PMU information to emev2.dtsi. With this >> > included KZM9D DT reference may use the PMU. >> >> Thanks, I will queue this up for v3.12 in the dt2 branch. > > When booting a debian armel userspace with this patch applied on top of > renesas-devel-20130731 I see the following. > > I do not observe this problem with the sh73a0 or r8a7740 versions > of this patch applied, though I assume that in the case of the r8a7740 > that is because it is UP. Looks like the STI clockevent rating patch is missing. SMP operation of EMEV2 is broken without that patch. Cheers, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 02/15] DMA: shdma: add r8a7740 DMAC data to the device ID table
Hi Guennadi, Thanks for your efforts on this. On Tue, Jul 23, 2013 at 7:49 PM, Guennadi Liakhovetski wrote: > This configuration data will be re-used, when DMAC DT support is added to > r8a7740, DMAC platform data in setup-r8a7740.c will be removed. > > Signed-off-by: Guennadi Liakhovetski > --- > > v4: make struct sh_dmae_pdata r8a7740_dma_pdata "const" > [snip] > --- /dev/null > +++ b/drivers/dma/sh/shdma-r8a7740.c > @@ -0,0 +1,95 @@ > +#include > + > +#include > +#include Including stuff from isn't really compatible with MULTIPLATFORM, so please don't write new code like this. Actually we don't want any code under drivers/ to include stuff from the mach directory. I suggest that you arrange your code in a way so the C version of DMAC support has tables with slave ids as usual under arch/arm/mach-shmobile/, but the DT bits that operate independently of C stay in drivers/... Over time we will get rid of the C version, and until that happens the DT and C version can coexist in parallel. Cheers, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ARM: shmobile: emev2: add PMU information to emev2.dtsi
From: Magnus Damm
Add PMU information to emev2.dtsi. With this
included KZM9D DT reference may use the PMU.
Signed-off-by: Magnus Damm
---
arch/arm/boot/dts/emev2.dtsi |6 ++
1 file changed, 6 insertions(+)
--- 0009/arch/arm/boot/dts/emev2.dtsi
+++ work/arch/arm/boot/dts/emev2.dtsi 2013-07-02 17:32:45.0 +0900
@@ -46,6 +46,12 @@
<0xe002 0x0100>;
};
+ pmu {
+ compatible = "arm,cortex-a9-pmu";
+ interrupts = <0 120 4>,
+<0 121 4>;
+ };
+
sti@e018 {
compatible = "renesas,em-sti";
reg = <0xe018 0x54>;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ARM: shmobile: sh73a0: add PMU information to sh73a0.dtsi
From: Magnus Damm
Add PMU information to sh73a0.dtsi. With this
included KZM9G DT reference may use the PMU.
Signed-off-by: Magnus Damm
---
arch/arm/boot/dts/sh73a0.dtsi |6 ++
1 file changed, 6 insertions(+)
--- 0001/arch/arm/boot/dts/sh73a0.dtsi
+++ work/arch/arm/boot/dts/sh73a0.dtsi 2013-07-24 04:12:57.0 +0900
@@ -38,6 +38,12 @@
<0xf100 0x100>;
};
+ pmu {
+ compatible = "arm,cortex-a9-pmu";
+ interrupts = <0 55 4>,
+<0 56 4>;
+ };
+
irqpin0: irqpin@e690 {
compatible = "renesas,intc-irqpin";
#interrupt-cells = <2>;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ARM: shmobile: r8a73a4: Remove ->init_machine() special case
From: Magnus Damm
No need to special case r8a73a4 ->init_machine(),
so get rid of undesired cpufreq platform device
from the generic long term r8a73a4 DT support code.
For short term support on APE6EVM the DT reference
implementation already adds a "cpufreq-cpu0" platform
device so that can be used for development.
Regarding more long term cpufreq support, perhaps
it makes sense to adjust the cpufreq driver to check
for DT information directly instead of using a
platform device for software configuration and DT
for hardware parameters.
Signed-off-by: Magnus Damm
---
arch/arm/mach-shmobile/setup-r8a73a4.c |6 --
1 file changed, 6 deletions(-)
--- 0001/arch/arm/mach-shmobile/setup-r8a73a4.c
+++ work/arch/arm/mach-shmobile/setup-r8a73a4.c 2013-07-23 16:22:29.0
+0900
@@ -215,11 +215,6 @@ void __init r8a73a4_init_delay(void)
}
#ifdef CONFIG_USE_OF
-void __init r8a73a4_add_standard_devices_dt(void)
-{
- platform_device_register_simple("cpufreq-cpu0", -1, NULL, 0);
- of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
-}
static const char *r8a73a4_boards_compat_dt[] __initdata = {
"renesas,r8a73a4",
@@ -228,7 +223,6 @@ static const char *r8a73a4_boards_compat
DT_MACHINE_START(R8A73A4_DT, "Generic R8A73A4 (Flattened Device Tree)")
.init_early = r8a73a4_init_delay,
- .init_machine = r8a73a4_add_standard_devices_dt,
.init_time = shmobile_timer_init,
.dt_compat = r8a73a4_boards_compat_dt,
MACHINE_END
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ARM: shmobile: r8a7740: add PMU information to r8a7740.dtsi
From: Magnus Damm
Add PMU information to r8a7740.dtsi. With this
included Armadillo800eva DT reference may use the PMU.
Signed-off-by: Magnus Damm
---
Dry coded based on data sheet, not runtime tested.
arch/arm/boot/dts/r8a7740.dtsi |5 +
1 file changed, 5 insertions(+)
--- 0001/arch/arm/boot/dts/r8a7740.dtsi
+++ work/arch/arm/boot/dts/r8a7740.dtsi 2013-07-24 04:20:42.0 +0900
@@ -32,6 +32,11 @@
<0xc200 0x1000>;
};
+ pmu {
+ compatible = "arm,cortex-a9-pmu";
+ interrupts = <0 83 4>;
+ };
+
/* irqpin0: IRQ0 - IRQ7 */
irqpin0: irqpin@e690 {
compatible = "renesas,intc-irqpin";
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 02/15] DMA: shdma: add r8a7740 DMAC data to the device ID table
Hi Guennadi, On Wed, Jul 24, 2013 at 6:19 AM, Guennadi Liakhovetski wrote: > On Wed, 24 Jul 2013, Magnus Damm wrote: > >> Hi Guennadi, >> >> Thanks for your efforts on this. >> >> On Tue, Jul 23, 2013 at 7:49 PM, Guennadi Liakhovetski >> wrote: >> > This configuration data will be re-used, when DMAC DT support is added to >> > r8a7740, DMAC platform data in setup-r8a7740.c will be removed. >> > >> > Signed-off-by: Guennadi Liakhovetski >> > --- >> > >> > v4: make struct sh_dmae_pdata r8a7740_dma_pdata "const" >> > >> >> [snip] >> >> > --- /dev/null >> > +++ b/drivers/dma/sh/shdma-r8a7740.c >> > @@ -0,0 +1,95 @@ >> > +#include >> > + >> > +#include >> > +#include >> >> Including stuff from isn't really compatible with >> MULTIPLATFORM, > > Hmm, right. I modeled this arch-specific driver code after Laurent's > pinctrl driver revamp, which also includes headers. So, we'll > have to think how to fix both. I mentioned this to Laurent when he started converting to PINCTRL, and I believe the only remaining bits are the static GPIO-to-IRQ tables. >> so please don't write new code like this. Actually we >> don't want any code under drivers/ to include stuff from the mach >> directory. > > Sure, understood. Good. >> I suggest that you arrange your code in a way so the C version of DMAC >> support has tables with slave ids as usual under >> arch/arm/mach-shmobile/, but the DT bits that operate independently of >> C stay in drivers/... Over time we will get rid of the C version, and >> until that happens the DT and C version can coexist in parallel. > > That's already how it is. Data, that I took to drivers/dma/sh/ is needed > for both DT and C. DMA stuff, needed only for C are only DMAC devices and > resources. I think, I might be able to carry those DMA specific headers > and defines over from mach/ to drivers/dma/sh. Maybe it would be easier to > do this in several steps: > > 1. add my drivers/dma/sh/shdma-.c files *with* mach/ headers > 2. switch arches over to those files > (the above two steps are already done in my patch series) > 3. move headers to drivers/dma/sh > > Ok, alternatively, I might be able to do (1) above without using mach/ > headers at all by directly copying them to drivers/dma/sh/ and then > removing the original mach/headers in step (2)? I'll look in more detail > at the code tomorrow. Thanks. My apologies for reviewing your code late in the cycle, but I've now looked through this series and the following questions popped up: 1) How will it look like in DT when a DMA Engine slave device will use DMA? 2) Isn't it possible to leave the SHDMA_SLAVE_xx bits to only be used by legacy C SoC and board code in arch/arm/mach-shmobile? I don't understand why you have to move them over to drivers/... I just assume these SHDMA_SLAVE bits won't be used by 1) above. 3) How difficult would it be to describe the information in "struct sh_dmae_slave_config" using DT? 4) It seems that some patches in this series are unrelated. Can you submit 4/15 and 9/15 from v4 independently somehow? 5) Reducing and extending (reducing is optional of course) At this point you cover r8a7740, r8a73a0 and r8a73a4. I'm not sure why your picked those 3 SoCs, but perhaps it would be a good idea to select a single SoC to begin with but extend the support to also provide DT code that makes use of DMA Engine in slave devices like for instance MMCIF (this would cover question 1) above). When we have agreed on the big picture then additional SoCs can easily be added later. 6) Remove untested bits And while doing this conversion, perhaps this is a good opportunity to only move over DMA Engine slaves that can be tested? Having tons of unused DMA configuration seems overly heavy to me. If it's not tested then it's broken. Cheers, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 02/15] DMA: shdma: add r8a7740 DMAC data to the device ID table
Hi Guennadi, On Wed, Jul 24, 2013 at 5:33 PM, Guennadi Liakhovetski wrote: > On Wed, 24 Jul 2013, Magnus Damm wrote: > >> Hi Guennadi, >> >> On Wed, Jul 24, 2013 at 6:19 AM, Guennadi Liakhovetski >> wrote: >> > On Wed, 24 Jul 2013, Magnus Damm wrote: >> > >> >> Hi Guennadi, >> >> >> >> Thanks for your efforts on this. >> >> >> >> On Tue, Jul 23, 2013 at 7:49 PM, Guennadi Liakhovetski >> >> wrote: >> >> > This configuration data will be re-used, when DMAC DT support is added >> >> > to >> >> > r8a7740, DMAC platform data in setup-r8a7740.c will be removed. >> >> > >> >> > Signed-off-by: Guennadi Liakhovetski >> >> > --- >> >> > >> >> > v4: make struct sh_dmae_pdata r8a7740_dma_pdata "const" >> >> > >> >> >> >> [snip] >> >> >> >> > --- /dev/null >> >> > +++ b/drivers/dma/sh/shdma-r8a7740.c >> >> > @@ -0,0 +1,95 @@ >> >> > +#include >> >> > + >> >> > +#include >> >> > +#include >> >> >> >> Including stuff from isn't really compatible with >> >> MULTIPLATFORM, >> > >> > Hmm, right. I modeled this arch-specific driver code after Laurent's >> > pinctrl driver revamp, which also includes headers. So, we'll >> > have to think how to fix both. >> >> I mentioned this to Laurent when he started converting to PINCTRL, and >> I believe the only remaining bits are the static GPIO-to-IRQ tables. >> >> >> so please don't write new code like this. Actually we >> >> don't want any code under drivers/ to include stuff from the mach >> >> directory. >> > >> > Sure, understood. >> >> Good. >> >> >> I suggest that you arrange your code in a way so the C version of DMAC >> >> support has tables with slave ids as usual under >> >> arch/arm/mach-shmobile/, but the DT bits that operate independently of >> >> C stay in drivers/... Over time we will get rid of the C version, and >> >> until that happens the DT and C version can coexist in parallel. >> > >> > That's already how it is. Data, that I took to drivers/dma/sh/ is needed >> > for both DT and C. DMA stuff, needed only for C are only DMAC devices and >> > resources. I think, I might be able to carry those DMA specific headers >> > and defines over from mach/ to drivers/dma/sh. Maybe it would be easier to >> > do this in several steps: >> > >> > 1. add my drivers/dma/sh/shdma-.c files *with* mach/ headers >> > 2. switch arches over to those files >> > (the above two steps are already done in my patch series) >> > 3. move headers to drivers/dma/sh >> > >> > Ok, alternatively, I might be able to do (1) above without using mach/ >> > headers at all by directly copying them to drivers/dma/sh/ and then >> > removing the original mach/headers in step (2)? I'll look in more detail >> > at the code tomorrow. >> >> Thanks. My apologies for reviewing your code late in the cycle, but >> I've now looked through this series and the following questions popped >> up: >> >> 1) How will it look like in DT when a DMA Engine slave device will use DMA? > > You have already seen them by now, since you replied to that thread too, > but just for reference, e.g. for an MMCIF controller here > http://thread.gmane.org/gmane.linux.ports.sh.devel/25445/focus=25442 > > + dmas = <&dmac 0xd1 > + &dmac 0xd2>; > + dma-names = "tx", "rx"; Yes, thanks for the pointer. >> 2) Isn't it possible to leave the SHDMA_SLAVE_xx bits to only be used >> by legacy C SoC and board code in arch/arm/mach-shmobile? I don't >> understand why you have to move them over to drivers/... I just assume >> these SHDMA_SLAVE bits won't be used by 1) above. > > That's right, those macros aren't used by (1) above, i.e. they aren't used > in DT. But they are used pretty intensively internally by the driver. The > C code might be "legacy," but as far as I understand, SuperH will never go > to DT, so, as long as it has to be supported, we need to support that > mode. Same for ARM - IIUC, board-*.c (not -reference) files are also still > quite intensively developed and supported. So, it doesn't look like the C > versi
[PATCH] gpio: Renesas R-Car GPIO driver
From: Magnus Damm
This patch implements a GPIO driver for the R-Car series
of SoCs from Renesas. This driver is designed to be reusable
between multiple SoCs that share the same basic building block,
but so far it has only been used on R-Car H1 (r8a7779).
Each driver instance handles 32 GPIOs with individually
maskable IRQs. The driver operates on a single I/O memory
range and the 32 GPIOs are hooked up a single interrupt.
In the case of R-Car H1 either external IRQ pins or GPIOs
with interrupts can be used for on-board interupts. For
external IRQs 4 pins are supported, and in the case of GPIO
there are 202 GPIOS as 202 interrupts hooked up via 6 driver
instances and to the GIC and the Cortex-A9 Quad.
At this point this driver is interfacing as a regular
platform device driver. In the future DT support will be
submitted as an incremental feature patch.
Signed-off-by: Magnus Damm
---
drivers/gpio/Kconfig|6
drivers/gpio/Makefile |1
drivers/gpio/gpio-rcar.c| 406 +++
include/linux/platform_data/gpio-rcar.h | 29 ++
4 files changed, 442 insertions(+)
--- 0001/drivers/gpio/Kconfig
+++ work/drivers/gpio/Kconfig 2013-03-05 09:07:34.0 +0900
@@ -201,6 +201,12 @@ config GPIO_PXA
help
Say yes here to support the PXA GPIO device
+config GPIO_RCAR
+ tristate "Renesas R-Car GPIO"
+ depends on ARM
+ help
+ Say yes here to support GPIO on Renesas R-Car SoCs.
+
config GPIO_SPEAR_SPICS
bool "ST SPEAr13xx SPI Chip Select as GPIO support"
depends on PLAT_SPEAR
--- 0001/drivers/gpio/Makefile
+++ work/drivers/gpio/Makefile 2013-03-05 09:07:34.0 +0900
@@ -56,6 +56,7 @@ obj-$(CONFIG_GPIO_PL061) += gpio-pl061.o
obj-$(CONFIG_GPIO_PXA) += gpio-pxa.o
obj-$(CONFIG_GPIO_RC5T583) += gpio-rc5t583.o
obj-$(CONFIG_GPIO_RDC321X) += gpio-rdc321x.o
+obj-$(CONFIG_GPIO_RCAR)+= gpio-rcar.o
obj-$(CONFIG_PLAT_SAMSUNG) += gpio-samsung.o
obj-$(CONFIG_ARCH_SA1100) += gpio-sa1100.o
obj-$(CONFIG_GPIO_SCH) += gpio-sch.o
--- /dev/null
+++ work/drivers/gpio/gpio-rcar.c 2013-03-05 09:13:23.0 +0900
@@ -0,0 +1,406 @@
+/*
+ * Renesas R-Car GPIO Support
+ *
+ * Copyright (C) 2013 Magnus Damm
+ *
+ * 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
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct gpio_rcar_priv {
+ void __iomem *base;
+ spinlock_t lock;
+ struct gpio_rcar_config config;
+ struct platform_device *pdev;
+ struct gpio_chip gpio_chip;
+ struct irq_chip irq_chip;
+ struct irq_domain *irq_domain;
+};
+
+#define IOINTSEL 0x00
+#define INOUTSEL 0x04
+#define OUTDT 0x08
+#define INDT 0x0c
+#define INTDT 0x10
+#define INTCLR 0x14
+#define INTMSK 0x18
+#define MSKCLR 0x1c
+#define POSNEG 0x20
+#define EDGLEVEL 0x24
+#define FILONOFF 0x28
+
+static inline unsigned long gpio_rcar_read(struct gpio_rcar_priv *p, int offs)
+{
+ return ioread32(p->base + offs);
+}
+
+static inline void gpio_rcar_write(struct gpio_rcar_priv *p, int offs,
+ unsigned long value)
+{
+ iowrite32(value, p->base + offs);
+}
+
+static void gpio_rcar_irq_disable(struct irq_data *d)
+{
+ struct gpio_rcar_priv *p = irq_data_get_irq_chip_data(d);
+
+ gpio_rcar_write(p, INTMSK, ~BIT(irqd_to_hwirq(d)));
+}
+
+static void gpio_rcar_irq_enable(struct irq_data *d)
+{
+ struct gpio_rcar_priv *p = irq_data_get_irq_chip_data(d);
+
+ gpio_rcar_write(p, MSKCLR, BIT(irqd_to_hwirq(d)));
+}
+
+static void gpio_rcar_config_interrupt_input_mode(struct gpio_rcar_priv *p,
+ unsigned int hwirq,
+ bool active_high_rising_edge,
+ bool level_trigger)
+{
+ unsigned long bit = BIT(hwirq);
+ unsigned long flags, tmp;
+
+ /* follow steps in the GPIO documentation for
+* "Setting Edge-Sensitive Interrupt Input Mode" and
+* "Setting Level-Sensitive Interrupt Input Mode"
+
[PATCH] irqchip: intc-irqpin: Initial DT support
From: Magnus Damm
Add initial DT support to the INTC External IRQ Pin
driver. At this point only hardware with 4-bit wide
sense registers is supported via DT.
Signed-off-by: Magnus Damm
---
drivers/irqchip/irq-renesas-intc-irqpin.c |9 +
1 file changed, 9 insertions(+)
--- 0001/drivers/irqchip/irq-renesas-intc-irqpin.c
+++ work/drivers/irqchip/irq-renesas-intc-irqpin.c 2013-03-06
14:59:58.0 +0900
@@ -278,6 +278,7 @@ static int intc_irqpin_irq_domain_map(st
static struct irq_domain_ops intc_irqpin_irq_domain_ops = {
.map= intc_irqpin_irq_domain_map,
+ .xlate = irq_domain_xlate_twocell,
};
static int intc_irqpin_probe(struct platform_device *pdev)
@@ -437,11 +438,19 @@ static int intc_irqpin_remove(struct pla
return 0;
}
+static const struct of_device_id intc_irqpin_dt_ids[] = {
+ { .compatible = "renesas,intc-irqpin", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, intc_irqpin_dt_ids);
+
static struct platform_driver intc_irqpin_device_driver = {
.probe = intc_irqpin_probe,
.remove = intc_irqpin_remove,
.driver = {
.name = "renesas_intc_irqpin",
+ .of_match_table = intc_irqpin_dt_ids,
+ .owner = THIS_MODULE,
}
};
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] irqchip: irqc: Add DT support
From: Magnus Damm
Add DT support to the IRQC External IRQ Pin driver.
Signed-off-by: Magnus Damm
---
drivers/irqchip/irq-renesas-irqc.c |9 +
1 file changed, 9 insertions(+)
--- 0001/drivers/irqchip/irq-renesas-irqc.c
+++ work/drivers/irqchip/irq-renesas-irqc.c 2013-03-06 15:01:42.0
+0900
@@ -145,6 +145,7 @@ static int irqc_irq_domain_map(struct ir
static struct irq_domain_ops irqc_irq_domain_ops = {
.map= irqc_irq_domain_map,
+ .xlate = irq_domain_xlate_twocell,
};
static int irqc_probe(struct platform_device *pdev)
@@ -273,11 +274,19 @@ static int irqc_remove(struct platform_d
return 0;
}
+static const struct of_device_id irqc_dt_ids[] = {
+ { .compatible = "renesas,irqc", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, irqc_dt_ids);
+
static struct platform_driver irqc_device_driver = {
.probe = irqc_probe,
.remove = irqc_remove,
.driver = {
.name = "renesas_irqc",
+ .of_match_table = irqc_dt_ids,
+ .owner = THIS_MODULE,
}
};
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] irqchip: Renesas INTC External IRQ pin driver
On Wed, Feb 27, 2013 at 7:28 PM, Paul Mundt wrote: > On Wed, Feb 27, 2013 at 06:52:51PM +0900, Magnus Damm wrote: >> As you know, the INTC code that you are referring to is a full >> interrupt controller designed to work directly with CPU cores like SH >> and ARM. Newer ARM cores like Cortex-A9 all include the GIC both for >> IPI purpose in case of SMP and they also implement SPI functionality >> for I/O device interrupts. So the need for vendor-specific interrupt >> controller IP is almost down to nothing these days. >> > Yes, I'm aware of that. Good! >> With that in mind I do really doubt that we end up reimplementing >> sh_intc. The upcoming designs that I am aware of do not change their >> external IRQ pin hardware to force us to update this driver. What >> happens after that I'm afraid I can't say. =) >> > While I don't expect you would end up with a full reimplmentation, my > concern is more that it would just be reproducing stuff that's already in > place. IOW, perhaps it's less work to put sh_intc on a diet, and tie the > core functionality that you need for external IRQ pins in to an irqchip > there -- with the core of the old code adapted in to some sort of common > base library, rather than coming up with a new lightweight irqchip driver > and having to incrementally pile stuff on top of it later. I'm afraid that I can't really see how we would want to pile that much stuff on top of this driver in the future. Maybe we need to adjust it slightly, but that should be fairly easy. >From my point of view it all boils down to this for our ARM SoCs: 1) The driver is only suitable for older GIC-based designs coming from the Renesas side. 2) GIC-based SoCs like EMEV2 coming from ex-NEC is using different IP and different driver. 3) Older SoCs that do not make use of GIC are using the full blown INTC implementation. >> I mainly wrote this driver to support the r8a7779 SoC which can't be >> driven by the existing INTC code base. During development I decided to >> try to share the driver between multiple GIC-based SoCs like r8a7779 >> and sh73a0. The reason behind trying to share this driver between >> multiple SoCs is to remove SoC-specific hacks that can't be dealt with >> by the existing INTC code. >> > Ok, fair enough. > >> I don't really understand why you would want to use a generic table >> driven driver when you have the possibility of using a >> hardware-specific driver. > > For the same reason sh_intc was written in the first place, every CPU > subtype more or less had a similar set of interrupt controllers with > minor deviations. Those deviations were sufficient enough to make the > code pretty hairy in places, and what we have now is really more of a > subsystem than an irqchip driver. > > Having to reproduce 90% of the code when you're adding new CPUs at the > rate of 2 a month hardly makes an SoC-specific driver realistic, > especially as you just end up with identical features being broken in > subtly differnt ways on different SoCs. That being said, a lot of that is > legacy stuff and a result of no CPU team talking to any other CPU team > ever, and it seems like things have improved enough in that regard that > perhaps a hardware-specific driver won't be a complete throw-away > disaster like it was before. It's a risk either way, I just hope your > lightweight solution remains lightweight and consistent long enough that > we don't have multiple copies of slightly different drivers doing exactly > same thing spiralling out of control and turning in to a maintenance > nightmare. Yes, I agree about the history with zillions of different SoCs, and the INTC and PFC design choice. But regarding the external pins in case of GIC-based designs, this seems to be something we can reuse as a regular driver. > If sh_intc doesn't deal with the needs of the new GIC-backed SoCs then > that's of course something that has to be addressed regardless (whether > that be hacking up sh_intc or adding new hardware-specific irqchips). Ignoring things won't work, agreed. >> I suppose you are wondering why no one seems to be working on INTC DT >> support at this point. The truth is that we got very little feedback >> and development support with interrupts in general last autumn and on >> top of that our developers went their own way instead of following >> advice. >> > I assumed it was either being rewritten or had already been merged, so I > was simply surprised to hear otherwise. I will dig through the archives a > bit later and see about getting caught up. Sounds good. Thanks, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/9] pwm: Add Renesas Mobile TPU PWM driver
Hi Laurent, On Wed, Apr 24, 2013 at 1:06 AM, Laurent Pinchart wrote: > The Timer Pulse Unit (TPU is a 4-channels 16-bit timer used to generate > waveforms. This driver exposes PWM functions through the PWM API for > other drivers to use. > > The code is loosely based on the leds-renesas-tpu driver by Magnus Damm > and the TPU PWM driver shipped in the Armadillo EVA 800 kernel sources. > > Signed-off-by: Laurent Pinchart > Tested-by: Simon Horman > --- > drivers/pwm/Kconfig| 7 + > drivers/pwm/Makefile | 1 + > drivers/pwm/pwm-rmob.c | 462 > + > include/linux/platform_data/pwm-rmob.h | 18 ++ > 4 files changed, 488 insertions(+) > create mode 100644 drivers/pwm/pwm-rmob.c > create mode 100644 include/linux/platform_data/pwm-rmob.h Thanks for your efforts with this driver. The new code becomes much nicer compared to my old driver. The TPU hardware block is used across several different Renesas product lines, including R-Mobile and R-Car. So with that in mind, can you please consider renaming this driver? I propose pwm-renesas-tpu.c or pwm-tpu.c instead of pwm-rmob.c. This because this driver has nothing to do with the R mobile product line. Thanks, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] gpio: em: Make use of devm functions
From: Magnus Damm
Update the Emma Mobile GPIO driver to make use of devm
functions. This simplifies the error handling and makes
the code more compact.
Signed-off-by: Magnus Damm
---
Applies to linux-next tag next-20130327, written on top of:
[PATCH] gpio: em: Add Device Tree support
drivers/gpio/gpio-em.c | 53 +---
1 file changed, 19 insertions(+), 34 deletions(-)
--- 0002/drivers/gpio/gpio-em.c
+++ work/drivers/gpio/gpio-em.c 2013-03-13 18:52:14.0 +0900
@@ -245,7 +245,7 @@ static int em_gio_probe(struct platform_
const char *name = dev_name(&pdev->dev);
int ret;
- p = kzalloc(sizeof(*p), GFP_KERNEL);
+ p = devm_kzalloc(&pdev->dev, sizeof(*p), GFP_KERNEL);
if (!p) {
dev_err(&pdev->dev, "failed to allocate driver data\n");
ret = -ENOMEM;
@@ -264,21 +264,23 @@ static int em_gio_probe(struct platform_
if (!io[0] || !io[1] || !irq[0] || !irq[1]) {
dev_err(&pdev->dev, "missing IRQ or IOMEM\n");
ret = -EINVAL;
- goto err1;
+ goto err0;
}
- p->base0 = ioremap_nocache(io[0]->start, resource_size(io[0]));
+ p->base0 = devm_ioremap_nocache(&pdev->dev, io[0]->start,
+ resource_size(io[0]));
if (!p->base0) {
dev_err(&pdev->dev, "failed to remap low I/O memory\n");
ret = -ENXIO;
- goto err1;
+ goto err0;
}
- p->base1 = ioremap_nocache(io[1]->start, resource_size(io[1]));
+ p->base1 = devm_ioremap_nocache(&pdev->dev, io[1]->start,
+ resource_size(io[1]));
if (!p->base1) {
dev_err(&pdev->dev, "failed to remap high I/O memory\n");
ret = -ENXIO;
- goto err2;
+ goto err0;
}
if (!pdata) {
@@ -289,13 +291,13 @@ static int em_gio_probe(struct platform_
&pdata->number_of_pins)) {
dev_err(&pdev->dev, "Missing ngpios OF property\n");
ret = -EINVAL;
- goto err3;
+ goto err0;
}
ret = of_alias_get_id(pdev->dev.of_node, "gpio");
if (ret < 0) {
dev_err(&pdev->dev, "Couldn't get OF id\n");
- goto err3;
+ goto err0;
}
pdata->gpio_base = ret * 32; /* 32 GPIOs per instance */
}
@@ -327,40 +329,32 @@ static int em_gio_probe(struct platform_
if (!p->irq_domain) {
ret = -ENXIO;
dev_err(&pdev->dev, "cannot initialize irq domain\n");
- goto err3;
+ goto err0;
}
- if (request_irq(irq[0]->start, em_gio_irq_handler, 0, name, p)) {
+ if (devm_request_irq(&pdev->dev, irq[0]->start,
+em_gio_irq_handler, 0, name, p)) {
dev_err(&pdev->dev, "failed to request low IRQ\n");
ret = -ENOENT;
- goto err4;
+ goto err1;
}
- if (request_irq(irq[1]->start, em_gio_irq_handler, 0, name, p)) {
+ if (devm_request_irq(&pdev->dev, irq[1]->start,
+em_gio_irq_handler, 0, name, p)) {
dev_err(&pdev->dev, "failed to request high IRQ\n");
ret = -ENOENT;
- goto err5;
+ goto err1;
}
ret = gpiochip_add(gpio_chip);
if (ret) {
dev_err(&pdev->dev, "failed to add GPIO controller\n");
- goto err6;
+ goto err1;
}
return 0;
-err6:
- free_irq(irq[1]->start, pdev);
-err5:
- free_irq(irq[0]->start, pdev);
-err4:
- irq_domain_remove(p->irq_domain);
-err3:
- iounmap(p->base1);
-err2:
- iounmap(p->base0);
err1:
- kfree(p);
+ irq_domain_remove(p->irq_domain);
err0:
return ret;
}
@@ -368,22 +362,13 @@ err0:
static int em_gio_remove(struct platform_device *pdev)
{
struct em_gio_priv *p = platform_get_drvdata(pdev);
- struct resource *irq[2];
int ret;
ret = gpiochip_remove(&p->gpio_chip);
if (ret)
return ret;
- irq[0] = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
- irq[1] = platform_get_resource(pdev, IORESOURCE_IRQ, 1);
-
- free_irq(irq[1]->start, pdev);
- free_irq(irq[0]->start, pdev);
irq_domain_remove(p->irq_
Re: [PATCH] gpio: em: Make use of devm functions
Hi Linus, On Wed, Mar 27, 2013 at 5:28 PM, Linus Walleij wrote: > On Wed, Mar 13, 2013 at 12:06 PM, Magnus Damm wrote: > >> From: Magnus Damm >> >> Update the Emma Mobile GPIO driver to make use of devm >> functions. This simplifies the error handling and makes >> the code more compact. >> >> Signed-off-by: Magnus Damm >> --- >> >> Written on top of: >> [PATCH] gpio: em: Add Device Tree support > > Patch applied, hm git am complains that the patch is broken > on line 137, then when I use patch -p1 < magnus.patch > everything works fine. Weird. Yes, indeed a bit weird. I actually tested the code about an hour ago, but I used patch then. Thanks for your help! / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 00/03] gpio: Renesas R-Car GPIO driver update
On Wed, Mar 27, 2013 at 9:34 PM, Linus Walleij wrote: > On Tue, Mar 19, 2013 at 4:36 AM, Magnus Damm wrote: >> On Thu, Mar 14, 2013 at 10:13 PM, Laurent Pinchart >> wrote: > >>> When submitting new drivers I usually try not to make the development >>> history >>> visible to mainline. It brings little additional value (beside possibly >>> making >>> backporting a bit easier, but in the devm_* case that shouldn't be a >>> problem, >>> unless Simon thinks otherwise) but adds review complexity, as reviewers need >>> to validate the intermediate versions as well. More patches also mean more >>> potential bisection breakages. >> >> Huh, it seems that my point of view is the total opposite. I see that >> using incremental patches to show new development would make review >> _easier_. Perhaps that's not the case for most people. > > As subsystem maintainer what I don't want to see is a patch that > at one point breaks something in some configuration and then later > fixes it. Then I strongly prefer squashing. (Greg also mentions this > in one of his seminars.) Sure, I totally agree. I strongly dislike when people introduce breakage and then fix it later in the same series. It's pretty obvious to me, each incremental step needs to work by itself - if it doesn't then it should be reworked before it gets merged. I personally prefer to separate features from fixes. Fixes are always folded into the original patch. > What really makes me mad is the the above pattern + claim that > it must be done in that way to preserve authorship. Like legaleaze > or credit is more important that functionality. > > If all patches are bisectable and perfectly fine then whether you > take 8 stops when driving to Rome or just drive there in one > big stretch is more a technical, secondary thing. Do whatever you > like as long as all commits build and boot. I suppose I prefer to stop for coffee in every village then. =) / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio: em: Make use of devm functions
On Thu, Mar 28, 2013 at 12:01 AM, Linus Walleij wrote: > On Wed, Mar 27, 2013 at 7:11 AM, Magnus Damm wrote: > >> From: Magnus Damm >> >> Update the Emma Mobile GPIO driver to make use of devm >> functions. This simplifies the error handling and makes >> the code more compact. >> >> Signed-off-by: Magnus Damm > > I guess this is a resend. IIRC I just applied it some hours ago... Yep, correct. Thanks for your help! Now if I only could find time to marry the gpio-em.c driver with sh-pfc.c.. / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio: Renesas R-Car GPIO driver V3
On Thu, Mar 28, 2013 at 12:16 AM, Linus Walleij wrote: > On Tue, Mar 26, 2013 at 1:23 AM, Magnus Damm wrote: > >> From: Magnus Damm >> >> This patch is V3 of a GPIO driver for the R-Car series of >> SoCs from Renesas. This driver is designed to be reusable >> between multiple SoCs that share the same basic building block, >> but so far it has only been used on R-Car H1 (r8a7779). >> >> Each driver instance handles 32 GPIOs with individually >> maskable IRQs. The driver operates on a single I/O memory >> range and the 32 GPIOs are hooked up a single interrupt. >> >> In the case of R-Car H1 either external IRQ pins or GPIOs >> with interrupts can be used for on-board interupts. For >> external IRQs 4 pins are supported, and in the case of GPIO >> there are 202 GPIOS as 202 interrupts hooked up via 6 driver >> instances and to the GIC and the Cortex-A9 Quad. >> >> At this point this driver is interfacing as a regular >> platform device driver. In the future DT support will be >> submitted as an incremental feature patch. >> >> Signed-off-by: Magnus Damm > > Gorgeous driver. > Reviewed-by: Linus Walleij Thanks! This particular GPIO hardware block hasn't been around for so long so it's still not spoiled by special cases like the PFC or INTC stuff. Cheers, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio: Renesas R-Car GPIO driver
HI Laurent,
On Sun, Mar 10, 2013 at 3:16 AM, Laurent Pinchart
wrote:
> Hi Magnus,
>
> Thank you for the patch.
Thanks for your review!
> On Tuesday 05 March 2013 09:32:19 Magnus Damm wrote:
>> From: Magnus Damm
>>
>> This patch implements a GPIO driver for the R-Car series
>> of SoCs from Renesas. This driver is designed to be reusable
>> between multiple SoCs that share the same basic building block,
>> but so far it has only been used on R-Car H1 (r8a7779).
>>
>> Each driver instance handles 32 GPIOs with individually
>> maskable IRQs. The driver operates on a single I/O memory
>> range and the 32 GPIOs are hooked up a single interrupt.
>>
>> In the case of R-Car H1 either external IRQ pins or GPIOs
>> with interrupts can be used for on-board interupts. For
>> external IRQs 4 pins are supported, and in the case of GPIO
>> there are 202 GPIOS as 202 interrupts hooked up via 6 driver
>> instances and to the GIC and the Cortex-A9 Quad.
>>
>> At this point this driver is interfacing as a regular
>> platform device driver. In the future DT support will be
>> submitted as an incremental feature patch.
>>
>> Signed-off-by: Magnus Damm
>> ---
>>
>> drivers/gpio/Kconfig|6
>> drivers/gpio/Makefile |1
>> drivers/gpio/gpio-rcar.c| 406
>> include/linux/platform_data/gpio-rcar.h | 29 ++
>> 4 files changed, 442 insertions(+)
>>
>> --- 0001/drivers/gpio/Kconfig
>> +++ work/drivers/gpio/Kconfig 2013-03-05 09:07:34.0 +0900
>> @@ -201,6 +201,12 @@ config GPIO_PXA
>> help
>> Say yes here to support the PXA GPIO device
>>
>> +config GPIO_RCAR
>> + tristate "Renesas R-Car GPIO"
>> + depends on ARM
>> + help
>> + Say yes here to support GPIO on Renesas R-Car SoCs.
>> +
>> config GPIO_SPEAR_SPICS
>> bool "ST SPEAr13xx SPI Chip Select as GPIO support"
>> depends on PLAT_SPEAR
>> --- 0001/drivers/gpio/Makefile
>> +++ work/drivers/gpio/Makefile2013-03-05 09:07:34.0 +0900
>> @@ -56,6 +56,7 @@ obj-$(CONFIG_GPIO_PL061)+= gpio-pl061.o
>> obj-$(CONFIG_GPIO_PXA) += gpio-pxa.o
>> obj-$(CONFIG_GPIO_RC5T583) += gpio-rc5t583.o
>> obj-$(CONFIG_GPIO_RDC321X) += gpio-rdc321x.o
>> +obj-$(CONFIG_GPIO_RCAR) += gpio-rcar.o
>> obj-$(CONFIG_PLAT_SAMSUNG) += gpio-samsung.o
>> obj-$(CONFIG_ARCH_SA1100)+= gpio-sa1100.o
>> obj-$(CONFIG_GPIO_SCH) += gpio-sch.o
>> --- /dev/null
>> +++ work/drivers/gpio/gpio-rcar.c 2013-03-05 09:13:23.0 +0900
>> @@ -0,0 +1,406 @@
>> +/*
>> + * Renesas R-Car GPIO Support
>> + *
>> + * Copyright (C) 2013 Magnus Damm
>> + *
>> + * 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
>> + *
>> + * 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.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
>> USA + */
>
> You can remove the last paragraph.
Sure!
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>
> Could you please sort the headers alphabetically ?
Uhm, ok, if that makes you happy! =)
>> +
>> +struct gpio_rcar_priv {
>> + void __iomem *base;
>> + spinlock_t lock;
>> + struct gpio_rcar_config config;
>> + struct platform_device *pdev;
>> + struct gpio_chip gpio_chip;
>> + struct irq_chip irq_chip;
>> + struct irq_domain *irq_domain;
>> +};
>> +
>> +#define IOINTSEL 0x00
>> +#define INOUTSEL 0x04
>> +#define OUTDT 0x08
>> +#define INDT 0x0c
>> +#define INTDT 0x10
>> +#define INTCLR 0x14
>> +#define INTMSK 0x18
>> +#define MSKCLR 0x1c
>> +#define POSNEG 0x20
>> +#defin
[PATCH] script: dtc: clean generated files
From: Magnus Damm Fix "make distclean" to clean up generated dtc files. Without this patch the following files are left around: - dtc-lexer.lex.c - dtc-parser.tab.c - dtc-parser.tab.h Signed-off-by: Magnus Damm --- scripts/dtc/Makefile |2 ++ 1 file changed, 2 insertions(+) --- 0001/scripts/dtc/Makefile +++ work/scripts/dtc/Makefile 2012-08-28 07:45:25.0 +0900 @@ -27,3 +27,5 @@ HOSTCFLAGS_dtc-parser.tab.o := $(HOSTCFL # dependencies on generated files need to be listed explicitly $(obj)/dtc-lexer.lex.o: $(obj)/dtc-parser.tab.h +# generated files need to be cleaned explicitly +clean-files:= dtc-lexer.lex.c dtc-parser.tab.c dtc-parser.tab.h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Current git very broken on the Dreamcast
Hi Adrian, [fixed up Matsubara-sans address] On Feb 17, 2008 4:48 AM, Adrian McMenamin <[EMAIL PROTECTED]> wrote: > On Sat, 2008-02-16 at 18:38 +, Adrian McMenamin wrote: > > Will seek to bisect this, but I have just updated my sources to the > > latest git and it is not booting at all on the Dreamcast. > > > > With early printk on, I get nothing more than this before an instant > > reboot: > > > > [0.00] Linux version 2.6.25-rc2-10953-g52065cd > > ([EMAIL PROTECTED]) (gcc version 3.4.6) #511 PREEMPT Sat Feb 16 18:31:43 > > GMT 2008 > > [0.00] console [sercon0] enabled > > [0.00] Booting machvec: Sega Dreamcast > > > [EMAIL PROTECTED]:~/gdrom-dev$ git bisect good > e036eaa681a17f71b64f6d9040fe60623919 is first bad commit > commit e036eaa681a17f71b64f6d9040fe60623919 > Author: Magnus Damm <[EMAIL PROTECTED]> > Date: Thu Feb 14 13:52:43 2008 +0900 > > sh: use ctrl_in/out for on chip pci access > > This patch makes sure ctrl_inN/outN are used instead of inN/outN for > on chip > pci registers. Without this patch addresses may be adjusted using > the value > in generic_io_base. This patch makes it possible to set > generic_io_base and > have pci without reading and writing all over the place. > > Signed-off-by: Magnus Damm <[EMAIL PROTECTED]> > Acked-by: Katsuya MATSUBARA <[EMAIL PROTECTED]> > Signed-off-by: Paul Mundt <[EMAIL PROTECTED]> > > :04 04 17a9f8181301e3a082d8a1a2fdef9b13ff1185e4 > 87e4a2e912be56b0d12e8a92db9489d6615c31f7 M arch Thanks for tracking this down and sorry for breaking the dreamcast pci driver. Most code for the dreamcast currently do direct register access using ctrl_inN/outN or readN/writeN. Only a few places use inN/outN and depend on the value of generic_io_base. Doing inN/outN is ok (but outdated, use ioreadN/iowriteN instead) in a portable pci/isa driver, but it doesn't make sense for any dreamcast specific code to rely on generic_io_base. For the dreamcast we already know the address at compile time so doing adjustment during runtime is just unnecessary overhead. It is of course possible to revert the dreamcast-specific bits of the commit you pointed out, but I think the change for the dreamcast makes sense since it makes the code both faster and more uniform. However, at this point it is broken. Sorry about that. I just looked through all upstream dreamcast-specific code I could find and irq.c is now the only file that does inN/outN. I've attached a patch that makes the irq code independent of generic_io_base. There is also another patch attached that adjusts the addresses of the pci register. Together they should solve the problem you are seeing. Please try them on top of 2.6.25-rc2. With these patches io ports count from 0 -> 8k-1 instead of being a pointer. We currently rely on generic_io_base logic but that will be changed in the future. Please let me know the results. Thank you. / magnus linux-2.6.25-rc2-sh-dreamcast-irq-20080218.patch Description: Binary data linux-2.6.25-rc2-sh-dreamcast-pci-20080218b.patch Description: Binary data
[PATCH] sm501: Add uart support
This patch extends the sm501 mfd with 8250 uart support. We're currently
doing this in the board specific r2d-1 code already, but it would be nice to
do move things into the mfd since it's more chip specific than board specific.
Signed-off-by: Magnus Damm <[EMAIL PROTECTED]>
---
drivers/mfd/sm501.c | 86 ++-
include/linux/serial_8250.h |1
2 files changed, 70 insertions(+), 17 deletions(-)
--- 0001/drivers/mfd/sm501.c
+++ work/drivers/mfd/sm501.c2008-02-08 18:40:33.0 +0900
@@ -22,6 +22,7 @@
#include
#include
+#include
#include
@@ -651,13 +652,14 @@ static void sm501_device_release(struct
*/
static struct platform_device *
-sm501_create_subdev(struct sm501_devdata *sm,
- char *name, unsigned int res_count)
+sm501_create_subdev(struct sm501_devdata *sm, char *name,
+ unsigned int res_count, unsigned int platform_data_size)
{
struct sm501_device *smdev;
smdev = kzalloc(sizeof(struct sm501_device) +
- sizeof(struct resource) * res_count, GFP_KERNEL);
+ (sizeof(struct resource) * res_count) +
+ platform_data_size, GFP_KERNEL);
if (!smdev)
return NULL;
@@ -665,11 +667,15 @@ sm501_create_subdev(struct sm501_devdata
smdev->pdev.name = name;
smdev->pdev.id = sm->pdev_id;
- smdev->pdev.resource = (struct resource *)(smdev+1);
- smdev->pdev.num_resources = res_count;
-
smdev->pdev.dev.parent = sm->dev;
+ if (res_count) {
+ smdev->pdev.resource = (struct resource *)(smdev+1);
+ smdev->pdev.num_resources = res_count;
+ }
+ if (platform_data_size)
+ smdev->pdev.dev.platform_data = (void *)(smdev+1);
+
return &smdev->pdev;
}
@@ -757,7 +763,7 @@ static int sm501_register_usbhost(struct
{
struct platform_device *pdev;
- pdev = sm501_create_subdev(sm, "sm501-usb", 3);
+ pdev = sm501_create_subdev(sm, "sm501-usb", 3, 0);
if (!pdev)
return -ENOMEM;
@@ -768,12 +774,55 @@ static int sm501_register_usbhost(struct
return sm501_register_device(sm, pdev);
}
+static void sm501_setup_uart_data(struct sm501_devdata *sm,
+ struct plat_serial8250_port *uart_data,
+ unsigned int offset)
+{
+ uart_data->membase = sm->regs + offset;
+ uart_data->mapbase = sm->io_res->start + offset;
+ uart_data->iotype = UPIO_MEM;
+ uart_data->irq = sm->irq;
+ uart_data->flags = UPF_BOOT_AUTOCONF | UPF_SKIP_TEST | UPF_SHARE_IRQ;
+ uart_data->regshift = 2;
+ uart_data->uartclk = (9600 * 16);
+}
+
+static int sm501_register_uart(struct sm501_devdata *sm, int devices)
+{
+ struct platform_device *pdev;
+ struct plat_serial8250_port *uart_data;
+
+ pdev = sm501_create_subdev(sm, "serial8250", 0,
+ sizeof(struct plat_serial8250_port) * 3);
+ if (!pdev)
+ return -ENOMEM;
+
+ uart_data = pdev->dev.platform_data;
+
+ if (devices & SM501_USE_UART0) {
+ sm501_setup_uart_data(sm, uart_data++, 0x3);
+ sm501_unit_power(sm->dev, SM501_GATE_UART0, 1);
+ sm501_modify_reg(sm->dev, SM501_IRQ_MASK, 1 << 12, 0);
+ sm501_modify_reg(sm->dev, SM501_GPIO63_32_CONTROL, 0x01e0, 0);
+ }
+ if (devices & SM501_USE_UART1) {
+ sm501_setup_uart_data(sm, uart_data++, 0x30020);
+ sm501_unit_power(sm->dev, SM501_GATE_UART1, 1);
+ sm501_modify_reg(sm->dev, SM501_IRQ_MASK, 1 << 13, 0);
+ sm501_modify_reg(sm->dev, SM501_GPIO63_32_CONTROL, 0x1e00, 0);
+ }
+
+ pdev->id = PLAT8250_DEV_SM501;
+
+ return sm501_register_device(sm, pdev);
+}
+
static int sm501_register_display(struct sm501_devdata *sm,
resource_size_t *mem_avail)
{
struct platform_device *pdev;
- pdev = sm501_create_subdev(sm, "sm501-fb", 4);
+ pdev = sm501_create_subdev(sm, "sm501-fb", 4, 0);
if (!pdev)
return -ENOMEM;
@@ -891,6 +940,7 @@ static unsigned int sm501_mem_local[] =
static int sm501_init_dev(struct sm501_devdata *sm)
{
+ struct sm501_initdata *idata;
resource_size_t mem_avail;
unsigned long dramctrl;
unsigned long devid;
@@ -908,6 +958,9 @@ static int sm501_init_dev(struct sm501_d
return -EINVAL;
}
+ /* disable irqs */
+ writel(0, sm->regs + SM501_IRQ_MASK);
+
dramctrl = readl(sm->regs + SM501_DRAM_CONTROL);
mem_avail = sm501_mem_loc
Re: [PATCH] ARM: shmobile: Add A4S cpuidle state on sh7372
Hi Rafael,
Thanks for your patch. Please see below for my comment.
On Fri, Aug 17, 2012 at 5:10 AM, Rafael J. Wysocki wrote:
>
> Add a "C5" cpuidle state to the SH7372 SoC connected to the A4S power
> domain in such a way that A4S may be turned off by cpuidle if all
> I/O devices in that domain have been suspended (or do not have
> attached drivers).
>
> This requires some reorganization of the initialization of SH7372
> power management which affects the the boards based on it, Mackerel
> and AP4EVB.
>
> Signed-off-by: Rafael J. Wysocki
> ---
[snip]
> --- linux.orig/arch/arm/mach-shmobile/pm-sh7372.c
> +++ linux/arch/arm/mach-shmobile/pm-sh7372.c
> @@ -339,6 +339,21 @@ static void sh7372_enter_a3sm_common(int
> sh7372_set_reset_vector(__pa(sh7372_resume_core_standby_sysc));
> sh7372_enter_sysc(pllc0_on, 1 << 12);
> }
> +
> +static void sh7372_enter_a4s_common(int pllc0_on)
> +{
> + sh7372_intca_suspend();
> + sh7372_enter_sysc(pllc0_on, 1 << 10);
> + sh7372_intca_resume();
> +}
> +
> +static void sh7372_pm_setup_smfram(void)
> +{
> + memcpy((void *)SMFRAM, sh7372_resume_core_standby_sysc, 0x100);
> + sh7372_set_reset_vector(SMFRAM);
> +}
I believe the reset vector is being setup dynamically depending on the
sleep mode, so you probably want to move sh7372_set_reset_vector()
into sh7372_enter_a4s_common().
Cheers,
/ magnus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/4] dma: sh: provide a migration path for slave drivers to stop using .private
Hi Guennadi, [CC Paul] On Thu, Jul 5, 2012 at 1:17 AM, Guennadi Liakhovetski wrote: > This patch extends the sh dmaengine driver to support the preferred channel > selection and configuration method, instead of using the "private" field > from struct dma_chan. We add a standard filter function to be used by > slave drivers instead of implementing their own ones, and add support for > the DMA_SLAVE_CONFIG control operation, which must accompany the new > channel selection method. We still support the legacy .private channel > allocation method to cater for a smooth driver migration. > > Signed-off-by: Guennadi Liakhovetski > --- Thanks for your efforts on this. Something that caught my eye in this patch is this portion: +bool shdma_chan_filter(struct dma_chan *chan, void *arg); If we would use this function in our DMA Engine slave drivers (MMCIF, SDHI, SCIF, FSI, SIU and so on) then wouldn't we add a strict dependency on this symbol provided by this particular DMA Engine driver implementation for the DMAC hardware (that your patch modifies)? And what do we do if we want to use the same DMA Engine slave driver with a different DMA Engine driver implementation? >From my point of view, there must be some better way to not have such tight dependencies between the DMA Engine slave consumer and the DMA Engine driver. Not sure what that looks like though. This symbol dependency is pretty far from great IMO. Thanks, / magnus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] irqchip: Renesas INTC External IRQ pin driver
From: Magnus Damm
This patch adds a driver for external IRQ pins connected
to the INTC block on recent SoCs from Renesas.
The INTC hardware block usually contains a rather wide
range of features ranging from external IRQ pin handling
to legacy interrupt controller support. On older SoCs
the INTC is used as a general purpose interrupt controller
both for external IRQ pins and on-chip devices.
On more recent ARM based SoCs with Cortex-A9 the main
interrupt controller is the GIC, but IRQ trigger setup
still need to happen in the INTC hardware block.
This driver implements the glue code needed to configure
IRQ trigger and also handle mask/unmask and demux of
external IRQ pins hooked up from the INTC to the GIC.
Tested on sh73a0 and r8a7779. The hardware varies quite
a bit with SoC model, for instance register width and
bitfield widths vary wildly. The driver requires one GIC
SPI per external IRQ pin to operate. Each driver instance
will handle up to 8 external IRQ pins.
The SoCs using this driver are currently mainly used
together with regular platform devices so this driver
allows configuration via platform data to support things
like static interrupt base address. DT support will
be added incrementally in the not so distant future.
Signed-off-by: Magnus Damm
---
drivers/irqchip/Kconfig |4
drivers/irqchip/Makefile |1
drivers/irqchip/irq-renesas-intc-irqpin.c | 464 +
include/linux/platform_data/irq-renesas-intc-irqpin.h | 10
4 files changed, 479 insertions(+)
--- 0001/drivers/irqchip/Kconfig
+++ work/drivers/irqchip/Kconfig2013-02-18 18:28:22.0 +0900
@@ -25,6 +25,10 @@ config ARM_VIC_NR
The maximum number of VICs available in the system, for
power management.
+config RENESAS_INTC_IRQPIN
+ bool
+ select IRQ_DOMAIN
+
config VERSATILE_FPGA_IRQ
bool
select IRQ_DOMAIN
--- 0001/drivers/irqchip/Makefile
+++ work/drivers/irqchip/Makefile 2013-02-18 18:28:22.0 +0900
@@ -5,4 +5,5 @@ obj-$(CONFIG_ARCH_SUNXI)+= irq-sunxi.o
obj-$(CONFIG_ARCH_SPEAR3XX)+= spear-shirq.o
obj-$(CONFIG_ARM_GIC) += irq-gic.o
obj-$(CONFIG_ARM_VIC) += irq-vic.o
+obj-$(CONFIG_RENESAS_INTC_IRQPIN) += irq-renesas-intc-irqpin.o
obj-$(CONFIG_VERSATILE_FPGA_IRQ) += irq-versatile-fpga.o
--- /dev/null
+++ work/drivers/irqchip/irq-renesas-intc-irqpin.c 2013-02-18
21:06:32.0 +0900
@@ -0,0 +1,464 @@
+/*
+ * Renesas INTC External IRQ Pin Driver
+ *
+ * Copyright (C) 2013 Magnus Damm
+ *
+ * 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
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define INTC_IRQPIN_MAX 8 /* maximum 8 interrupts per driver instance */
+
+#define INTC_IRQPIN_REG_SENSE 0 /* ICRn */
+#define INTC_IRQPIN_REG_PRIO 1 /* INTPRInn */
+#define INTC_IRQPIN_REG_SOURCE 2 /* INTREQnn */
+#define INTC_IRQPIN_REG_MASK 3 /* INTMSKnn */
+#define INTC_IRQPIN_REG_CLEAR 4 /* INTMSKCLRnn */
+#define INTC_IRQPIN_REG_NR 5
+
+/* INTC external IRQ PIN hardware register access:
+ *
+ * SENSE is read-write 32-bit with 2-bits or 4-bits per IRQ (*)
+ * PRIO is read-write 32-bit with 4-bits per IRQ (**)
+ * SOURCE is read-only 32-bit or 8-bit with 1-bit per IRQ (***)
+ * MASK is write-only 32-bit or 8-bit with 1-bit per IRQ (***)
+ * CLEAR is write-only 32-bit or 8-bit with 1-bit per IRQ (***)
+ *
+ * (*) May be accessed by more than one driver instance - lock needed
+ * (**) Read-modify-write access by one driver instance - lock needed
+ * (***) Accessed by one driver instance only - no locking needed
+ */
+
+struct intc_irqpin_iomem {
+ void __iomem *iomem;
+ unsigned long (*read)(void __iomem *iomem);
+ void (*write)(void __iomem *iomem, unsigned long data);
+ int width;
+};
+
+struct intc_irqpin_irq {
+ int hw_irq;
+ int irq;
+ struct intc_irqpin_priv *p;
+};
+
+struct intc_irqpin_priv {
+ struct intc_irqpin_iomem iomem[INTC_IRQPIN_REG_NR];
+ struct intc_irqpin_irq irq[INTC_IRQPIN_MAX];
+ struct renesas_intc_irqpin_config config;
+ unsigned int number_of_irqs;
+ struct platform_device *pdev
[PATCH 00/08] clocksource: sh_cmt: CMT driver update
clocksource: sh_cmt: CMT driver update [PATCH 01/08] clocksource: sh_cmt: Take care of clk_put() when setup_irq() fails [PATCH 02/08] clocksource: sh_cmt: Initialize 'max_match_value' and 'lock' in sh_cmt_setup() [PATCH 03/08] clocksource: sh_cmt: Consolidate platform_set_drvdata() call [PATCH 04/08] clocksource: sh_cmt: Introduce per-register functions [PATCH 05/08] clocksource: sh_cmt: CMSTR and CMCSR register access update [PATCH 06/08] clocksource: sh_cmt: CMCNT and CMCOR register access update [PATCH 07/08] clocksource: sh_cmt: Add control register callbacks [PATCH 08/08] clocksource: sh_cmt: Add CMT register layout comment This patch series contains a couple of driver updates from Kuribayashi-san together with some register access changes from me. The register access changes work towards adding support for 32-bit only CMT hardware, but the final bits are not yet included in this series due to lack of hardware. Patches 1-3: Signed-off-by: Shinya Kuribayashi Signed-off-by: Magnus Damm Patches 4-8: Signed-off-by: Magnus Damm --- drivers/clocksource/sh_cmt.c | 257 -- 1 file changed, 152 insertions(+), 105 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 01/08] clocksource: sh_cmt: Take care of clk_put() when setup_irq() fails
From: Magnus Damm
Make sure clk_put() is called in case of failure in sh_cmt_setup().
Signed-off-by: Shinya Kuribayashi
Signed-off-by: Magnus Damm
---
drivers/clocksource/sh_cmt.c |6 --
1 file changed, 4 insertions(+), 2 deletions(-)
--- 0001/drivers/clocksource/sh_cmt.c
+++ work/drivers/clocksource/sh_cmt.c 2012-12-14 12:50:02.0 +0900
@@ -708,17 +708,19 @@ static int sh_cmt_setup(struct sh_cmt_pr
cfg->clocksource_rating);
if (ret) {
dev_err(&p->pdev->dev, "registration failed\n");
- goto err1;
+ goto err2;
}
p->cs_enabled = false;
ret = setup_irq(irq, &p->irqaction);
if (ret) {
dev_err(&p->pdev->dev, "failed to request irq %d\n", irq);
- goto err1;
+ goto err2;
}
return 0;
+err2:
+ clk_put(p->clk);
err1:
iounmap(p->mapbase);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 02/08] clocksource: sh_cmt: Initialize 'max_match_value' and 'lock' in sh_cmt_setup()
From: Magnus Damm
Move the setup of spinlock and max_match_value to sh_cmt_setup().
There's no need to defer those steps until sh_cmt_register().
Signed-off-by: Shinya Kuribayashi
Signed-off-by: Magnus Damm
---
drivers/clocksource/sh_cmt.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
--- 0002/drivers/clocksource/sh_cmt.c
+++ work/drivers/clocksource/sh_cmt.c 2012-12-14 12:52:35.0 +0900
@@ -625,14 +625,6 @@ static int sh_cmt_register(struct sh_cmt
unsigned long clockevent_rating,
unsigned long clocksource_rating)
{
- if (p->width == (sizeof(p->max_match_value) * 8))
- p->max_match_value = ~0;
- else
- p->max_match_value = (1 << p->width) - 1;
-
- p->match_value = p->max_match_value;
- raw_spin_lock_init(&p->lock);
-
if (clockevent_rating)
sh_cmt_register_clockevent(p, name, clockevent_rating);
@@ -703,6 +695,14 @@ static int sh_cmt_setup(struct sh_cmt_pr
p->clear_bits = ~0xc000;
}
+ if (p->width == (sizeof(p->max_match_value) * 8))
+ p->max_match_value = ~0;
+ else
+ p->max_match_value = (1 << p->width) - 1;
+
+ p->match_value = p->max_match_value;
+ raw_spin_lock_init(&p->lock);
+
ret = sh_cmt_register(p, (char *)dev_name(&p->pdev->dev),
cfg->clockevent_rating,
cfg->clocksource_rating);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 03/08] clocksource: sh_cmt: Consolidate platform_set_drvdata() call
From: Magnus Damm
Cleanup the use of platform_set_drvdata() to reduce code size
Signed-off-by: Shinya Kuribayashi
Signed-off-by: Magnus Damm
---
drivers/clocksource/sh_cmt.c |5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
--- 0003/drivers/clocksource/sh_cmt.c
+++ work/drivers/clocksource/sh_cmt.c 2012-12-14 12:54:59.0 +0900
@@ -649,8 +649,6 @@ static int sh_cmt_setup(struct sh_cmt_pr
goto err0;
}
- platform_set_drvdata(pdev, p);
-
res = platform_get_resource(p->pdev, IORESOURCE_MEM, 0);
if (!res) {
dev_err(&p->pdev->dev, "failed to get I/O memory\n");
@@ -718,6 +716,8 @@ static int sh_cmt_setup(struct sh_cmt_pr
goto err2;
}
+ platform_set_drvdata(pdev, p);
+
return 0;
err2:
clk_put(p->clk);
@@ -753,7 +753,6 @@ static int __devinit sh_cmt_probe(struct
ret = sh_cmt_setup(p, pdev);
if (ret) {
kfree(p);
- platform_set_drvdata(pdev, NULL);
pm_runtime_idle(&pdev->dev);
return ret;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 04/08] clocksource: sh_cmt: Introduce per-register functions
From: Magnus Damm
Introduce sh_cmt_read_cmstr/cmcsr/cmcnt() and
sh_cmt_write_cmstr/cmcsr/cmcnt/cmcor() to in the
future allow us to split counter registers from
control registers and reduce code complexity by
removing sh_cmt_read() and sh_cmt_write().
Signed-off-by: Magnus Damm
---
drivers/clocksource/sh_cmt.c | 71 --
1 file changed, 55 insertions(+), 16 deletions(-)
--- 0001/drivers/clocksource/sh_cmt.c
+++ work/drivers/clocksource/sh_cmt.c 2012-12-14 10:50:15.0 +0900
@@ -86,6 +86,21 @@ static inline unsigned long sh_cmt_read(
return ioread16(base + offs);
}
+static inline unsigned long sh_cmt_read_cmstr(struct sh_cmt_priv *p)
+{
+ return sh_cmt_read(p, CMSTR);
+}
+
+static inline unsigned long sh_cmt_read_cmcsr(struct sh_cmt_priv *p)
+{
+ return sh_cmt_read(p, CMCSR);
+}
+
+static inline unsigned long sh_cmt_read_cmcnt(struct sh_cmt_priv *p)
+{
+ return sh_cmt_read(p, CMCNT);
+}
+
static inline void sh_cmt_write(struct sh_cmt_priv *p, int reg_nr,
unsigned long value)
{
@@ -112,21 +127,45 @@ static inline void sh_cmt_write(struct s
iowrite16(value, base + offs);
}
+static inline void sh_cmt_write_cmstr(struct sh_cmt_priv *p,
+ unsigned long value)
+{
+ sh_cmt_write(p, CMSTR, value);
+}
+
+static inline void sh_cmt_write_cmcsr(struct sh_cmt_priv *p,
+ unsigned long value)
+{
+ sh_cmt_write(p, CMCSR, value);
+}
+
+static inline void sh_cmt_write_cmcnt(struct sh_cmt_priv *p,
+ unsigned long value)
+{
+ sh_cmt_write(p, CMCNT, value);
+}
+
+static inline void sh_cmt_write_cmcor(struct sh_cmt_priv *p,
+ unsigned long value)
+{
+ sh_cmt_write(p, CMCOR, value);
+}
+
static unsigned long sh_cmt_get_counter(struct sh_cmt_priv *p,
int *has_wrapped)
{
unsigned long v1, v2, v3;
int o1, o2;
- o1 = sh_cmt_read(p, CMCSR) & p->overflow_bit;
+ o1 = sh_cmt_read_cmcsr(p) & p->overflow_bit;
/* Make sure the timer value is stable. Stolen from acpi_pm.c */
do {
o2 = o1;
- v1 = sh_cmt_read(p, CMCNT);
- v2 = sh_cmt_read(p, CMCNT);
- v3 = sh_cmt_read(p, CMCNT);
- o1 = sh_cmt_read(p, CMCSR) & p->overflow_bit;
+ v1 = sh_cmt_read_cmcnt(p);
+ v2 = sh_cmt_read_cmcnt(p);
+ v3 = sh_cmt_read_cmcnt(p);
+ o1 = sh_cmt_read_cmcsr(p) & p->overflow_bit;
} while (unlikely((o1 != o2) || (v1 > v2 && v1 < v3)
|| (v2 > v3 && v2 < v1) || (v3 > v1 && v3 < v2)));
@@ -142,14 +181,14 @@ static void sh_cmt_start_stop_ch(struct
/* start stop register shared by multiple timer channels */
raw_spin_lock_irqsave(&sh_cmt_lock, flags);
- value = sh_cmt_read(p, CMSTR);
+ value = sh_cmt_read_cmstr(p);
if (start)
value |= 1 << cfg->timer_bit;
else
value &= ~(1 << cfg->timer_bit);
- sh_cmt_write(p, CMSTR, value);
+ sh_cmt_write_cmstr(p, value);
raw_spin_unlock_irqrestore(&sh_cmt_lock, flags);
}
@@ -173,14 +212,14 @@ static int sh_cmt_enable(struct sh_cmt_p
/* configure channel, periodic mode and maximum timeout */
if (p->width == 16) {
*rate = clk_get_rate(p->clk) / 512;
- sh_cmt_write(p, CMCSR, 0x43);
+ sh_cmt_write_cmcsr(p, 0x43);
} else {
*rate = clk_get_rate(p->clk) / 8;
- sh_cmt_write(p, CMCSR, 0x01a4);
+ sh_cmt_write_cmcsr(p, 0x01a4);
}
- sh_cmt_write(p, CMCOR, 0x);
- sh_cmt_write(p, CMCNT, 0);
+ sh_cmt_write_cmcor(p, 0x);
+ sh_cmt_write_cmcnt(p, 0);
/*
* According to the sh73a0 user's manual, as CMCNT can be operated
@@ -194,12 +233,12 @@ static int sh_cmt_enable(struct sh_cmt_p
* take RCLKx2 at maximum.
*/
for (k = 0; k < 100; k++) {
- if (!sh_cmt_read(p, CMCNT))
+ if (!sh_cmt_read_cmcnt(p))
break;
udelay(1);
}
- if (sh_cmt_read(p, CMCNT)) {
+ if (sh_cmt_read_cmcnt(p)) {
dev_err(&p->pdev->dev, "cannot clear CMCNT\n");
ret = -ETIMEDOUT;
goto err1;
@@ -222,7 +261,7 @@ static void sh_cmt_disable(struct sh_cmt
sh_cmt_start_stop_ch(p, 0);
/* disable interrupts in CMT block */
- sh_cmt_write(p, CMCSR, 0);
+ sh_cmt_write_cmcsr(p, 0);
/* stop clock */
clk_disable(p->clk);
@@ -270,7
[PATCH 05/08] clocksource: sh_cmt: CMSTR and CMCSR register access update
From: Magnus Damm
Update hardware register access code for CMSTR and CMCSR
from using sh_cmt_read() and sh_cmt_write() to make use
of 16-bit register access functions such as sh_cmt_read16()
and sh_cmt_write16(). Also update sh_cmt_read() and
sh_cmt_write() now when the special cases are gone.
This patch moves us one step closer to the goal of separating
counter register access functions from control control register
functions.
Signed-off-by: Magnus Damm
---
drivers/clocksource/sh_cmt.c | 66 +++---
1 file changed, 30 insertions(+), 36 deletions(-)
--- 0005/drivers/clocksource/sh_cmt.c
+++ work/drivers/clocksource/sh_cmt.c 2012-12-14 12:58:01.0 +0900
@@ -56,44 +56,46 @@ struct sh_cmt_priv {
bool cs_enabled;
};
-static DEFINE_RAW_SPINLOCK(sh_cmt_lock);
+static inline unsigned long sh_cmt_read16(void __iomem *base,
+ unsigned long offs)
+{
+ return ioread16(base + (offs << 1));
+}
+
+static inline void sh_cmt_write16(void __iomem *base, unsigned long offs,
+ unsigned long value)
+{
+ iowrite16(value, base + (offs << 1));
+}
-#define CMSTR -1 /* shared register */
#define CMCSR 0 /* channel register */
#define CMCNT 1 /* channel register */
#define CMCOR 2 /* channel register */
static inline unsigned long sh_cmt_read(struct sh_cmt_priv *p, int reg_nr)
{
- struct sh_timer_config *cfg = p->pdev->dev.platform_data;
void __iomem *base = p->mapbase;
- unsigned long offs;
+ unsigned long offs = reg_nr;
- if (reg_nr == CMSTR) {
- offs = 0;
- base -= cfg->channel_offset;
- } else
- offs = reg_nr;
-
- if (p->width == 16)
+ if (p->width == 16) {
offs <<= 1;
- else {
+ return ioread16(base + offs);
+ } else {
offs <<= 2;
- if ((reg_nr == CMCNT) || (reg_nr == CMCOR))
- return ioread32(base + offs);
+ return ioread32(base + offs);
}
-
- return ioread16(base + offs);
}
static inline unsigned long sh_cmt_read_cmstr(struct sh_cmt_priv *p)
{
- return sh_cmt_read(p, CMSTR);
+ struct sh_timer_config *cfg = p->pdev->dev.platform_data;
+
+ return sh_cmt_read16(p->mapbase - cfg->channel_offset, 0);
}
static inline unsigned long sh_cmt_read_cmcsr(struct sh_cmt_priv *p)
{
- return sh_cmt_read(p, CMCSR);
+ return sh_cmt_read16(p->mapbase, CMCSR);
}
static inline unsigned long sh_cmt_read_cmcnt(struct sh_cmt_priv *p)
@@ -104,39 +106,30 @@ static inline unsigned long sh_cmt_read_
static inline void sh_cmt_write(struct sh_cmt_priv *p, int reg_nr,
unsigned long value)
{
- struct sh_timer_config *cfg = p->pdev->dev.platform_data;
void __iomem *base = p->mapbase;
- unsigned long offs;
-
- if (reg_nr == CMSTR) {
- offs = 0;
- base -= cfg->channel_offset;
- } else
- offs = reg_nr;
+ unsigned long offs = reg_nr;
- if (p->width == 16)
+ if (p->width == 16) {
offs <<= 1;
- else {
+ iowrite16(value, base + offs);
+ } else {
offs <<= 2;
- if ((reg_nr == CMCNT) || (reg_nr == CMCOR)) {
- iowrite32(value, base + offs);
- return;
- }
+ iowrite32(value, base + offs);
}
-
- iowrite16(value, base + offs);
}
static inline void sh_cmt_write_cmstr(struct sh_cmt_priv *p,
unsigned long value)
{
- sh_cmt_write(p, CMSTR, value);
+ struct sh_timer_config *cfg = p->pdev->dev.platform_data;
+
+ sh_cmt_write16(p->mapbase - cfg->channel_offset, 0, value);
}
static inline void sh_cmt_write_cmcsr(struct sh_cmt_priv *p,
unsigned long value)
{
- sh_cmt_write(p, CMCSR, value);
+ sh_cmt_write16(p->mapbase, CMCSR, value);
}
static inline void sh_cmt_write_cmcnt(struct sh_cmt_priv *p,
@@ -173,6 +166,7 @@ static unsigned long sh_cmt_get_counter(
return v2;
}
+static DEFINE_RAW_SPINLOCK(sh_cmt_lock);
static void sh_cmt_start_stop_ch(struct sh_cmt_priv *p, int start)
{
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 06/08] clocksource: sh_cmt: CMCNT and CMCOR register access update
From: Magnus Damm
Break out the CMCNT and CMCOR register access code
into separate 16-bit and 32-bit functions that are
hooked into callbacks at init time. This reduces
the amount of software calculations happening at
runtime.
Signed-off-by: Magnus Damm
---
drivers/clocksource/sh_cmt.c | 62 +-
1 file changed, 26 insertions(+), 36 deletions(-)
--- 0006/drivers/clocksource/sh_cmt.c
+++ work/drivers/clocksource/sh_cmt.c 2012-12-14 13:00:20.0 +0900
@@ -54,38 +54,39 @@ struct sh_cmt_priv {
struct clocksource cs;
unsigned long total_cycles;
bool cs_enabled;
+
+ /* callbacks for CMCNT and CMCOR access */
+ unsigned long (*read_count)(void __iomem *base, unsigned long offs);
+ void (*write_count)(void __iomem *base, unsigned long offs,
+ unsigned long value);
};
-static inline unsigned long sh_cmt_read16(void __iomem *base,
- unsigned long offs)
+static unsigned long sh_cmt_read16(void __iomem *base, unsigned long offs)
{
return ioread16(base + (offs << 1));
}
-static inline void sh_cmt_write16(void __iomem *base, unsigned long offs,
- unsigned long value)
+static unsigned long sh_cmt_read32(void __iomem *base, unsigned long offs)
+{
+ return ioread32(base + (offs << 2));
+}
+
+static void sh_cmt_write16(void __iomem *base, unsigned long offs,
+ unsigned long value)
{
iowrite16(value, base + (offs << 1));
}
+static void sh_cmt_write32(void __iomem *base, unsigned long offs,
+ unsigned long value)
+{
+ iowrite32(value, base + (offs << 2));
+}
+
#define CMCSR 0 /* channel register */
#define CMCNT 1 /* channel register */
#define CMCOR 2 /* channel register */
-static inline unsigned long sh_cmt_read(struct sh_cmt_priv *p, int reg_nr)
-{
- void __iomem *base = p->mapbase;
- unsigned long offs = reg_nr;
-
- if (p->width == 16) {
- offs <<= 1;
- return ioread16(base + offs);
- } else {
- offs <<= 2;
- return ioread32(base + offs);
- }
-}
-
static inline unsigned long sh_cmt_read_cmstr(struct sh_cmt_priv *p)
{
struct sh_timer_config *cfg = p->pdev->dev.platform_data;
@@ -100,22 +101,7 @@ static inline unsigned long sh_cmt_read_
static inline unsigned long sh_cmt_read_cmcnt(struct sh_cmt_priv *p)
{
- return sh_cmt_read(p, CMCNT);
-}
-
-static inline void sh_cmt_write(struct sh_cmt_priv *p, int reg_nr,
- unsigned long value)
-{
- void __iomem *base = p->mapbase;
- unsigned long offs = reg_nr;
-
- if (p->width == 16) {
- offs <<= 1;
- iowrite16(value, base + offs);
- } else {
- offs <<= 2;
- iowrite32(value, base + offs);
- }
+ return p->read_count(p->mapbase, CMCNT);
}
static inline void sh_cmt_write_cmstr(struct sh_cmt_priv *p,
@@ -135,13 +121,13 @@ static inline void sh_cmt_write_cmcsr(st
static inline void sh_cmt_write_cmcnt(struct sh_cmt_priv *p,
unsigned long value)
{
- sh_cmt_write(p, CMCNT, value);
+ p->write_count(p->mapbase, CMCNT, value);
}
static inline void sh_cmt_write_cmcor(struct sh_cmt_priv *p,
unsigned long value)
{
- sh_cmt_write(p, CMCOR, value);
+ p->write_count(p->mapbase, CMCOR, value);
}
static unsigned long sh_cmt_get_counter(struct sh_cmt_priv *p,
@@ -718,10 +704,14 @@ static int sh_cmt_setup(struct sh_cmt_pr
if (resource_size(res) == 6) {
p->width = 16;
+ p->read_count = sh_cmt_read16;
+ p->write_count = sh_cmt_write16;
p->overflow_bit = 0x80;
p->clear_bits = ~0x80;
} else {
p->width = 32;
+ p->read_count = sh_cmt_read32;
+ p->write_count = sh_cmt_write32;
p->overflow_bit = 0x8000;
p->clear_bits = ~0xc000;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 07/08] clocksource: sh_cmt: Add control register callbacks
From: Magnus Damm
This patch adds control register callbacks for the CMT
driver. At this point only 16-bit access is supported
but in the future this will be updated to allow 32-bit
access as well.
Signed-off-by: Magnus Damm
---
drivers/clocksource/sh_cmt.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
--- 0004/drivers/clocksource/sh_cmt.c
+++ work/drivers/clocksource/sh_cmt.c 2012-12-14 11:28:57.0 +0900
@@ -55,6 +55,11 @@ struct sh_cmt_priv {
unsigned long total_cycles;
bool cs_enabled;
+ /* callbacks for CMSTR and CMCSR access */
+ unsigned long (*read_control)(void __iomem *base, unsigned long offs);
+ void (*write_control)(void __iomem *base, unsigned long offs,
+ unsigned long value);
+
/* callbacks for CMCNT and CMCOR access */
unsigned long (*read_count)(void __iomem *base, unsigned long offs);
void (*write_count)(void __iomem *base, unsigned long offs,
@@ -94,12 +99,12 @@ static inline unsigned long sh_cmt_read_
{
struct sh_timer_config *cfg = p->pdev->dev.platform_data;
- return sh_cmt_read16(p->mapbase - cfg->channel_offset, 0);
+ return p->read_control(p->mapbase - cfg->channel_offset, 0);
}
static inline unsigned long sh_cmt_read_cmcsr(struct sh_cmt_priv *p)
{
- return sh_cmt_read16(p->mapbase, CMCSR);
+ return p->read_control(p->mapbase, CMCSR);
}
static inline unsigned long sh_cmt_read_cmcnt(struct sh_cmt_priv *p)
@@ -112,13 +117,13 @@ static inline void sh_cmt_write_cmstr(st
{
struct sh_timer_config *cfg = p->pdev->dev.platform_data;
- sh_cmt_write16(p->mapbase - cfg->channel_offset, 0, value);
+ p->write_control(p->mapbase - cfg->channel_offset, 0, value);
}
static inline void sh_cmt_write_cmcsr(struct sh_cmt_priv *p,
unsigned long value)
{
- sh_cmt_write16(p->mapbase, CMCSR, value);
+ p->write_control(p->mapbase, CMCSR, value);
}
static inline void sh_cmt_write_cmcnt(struct sh_cmt_priv *p,
@@ -714,6 +719,9 @@ static int sh_cmt_setup(struct sh_cmt_pr
goto err1;
}
+ p->read_control = sh_cmt_read16;
+ p->write_control = sh_cmt_write16;
+
if (resource_size(res) == 6) {
p->width = 16;
p->read_count = sh_cmt_read16;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 08/08] clocksource: sh_cmt: Add CMT register layout comment
From: Magnus Damm
Add a comment about different register layouts
supported by the CMT driver.
Signed-off-by: Magnus Damm
---
drivers/clocksource/sh_cmt.c | 15 +++
1 file changed, 15 insertions(+)
--- 0005/drivers/clocksource/sh_cmt.c
+++ work/drivers/clocksource/sh_cmt.c 2012-12-14 11:43:10.0 +0900
@@ -66,6 +66,21 @@ struct sh_cmt_priv {
unsigned long value);
};
+/* Examples of supported CMT timer register layouts and I/O access widths:
+ *
+ * "16-bit counter and 16-bit control" as found on sh7263:
+ * CMSTR 0xfffec000 16-bit
+ * CMCSR 0xfffec002 16-bit
+ * CMCNT 0xfffec004 16-bit
+ * CMCOR 0xfffec006 16-bit
+ *
+ * "32-bit counter and 16-bit control" as found on sh7372, sh73a0, r8a7740:
+ * CMSTR 0xffca 16-bit
+ * CMCSR 0xffca0060 16-bit
+ * CMCNT 0xffca0064 32-bit
+ * CMCOR 0xffca0068 32-bit
+ */
+
static unsigned long sh_cmt_read16(void __iomem *base, unsigned long offs)
{
return ioread16(base + (offs << 1));
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
2.2.18 corruption: IDE + PCMCIA ?
Hi all, I've experienced some disk corruption on my laptop. Scenario: I'm cross-compiling tons of sources and I felt the need to insert a CompactFlash card (via PCMCIA) in my laptop. So I did, no problem: mounted, touched a file, umounted, cardctl-ejected. Pretty soon my compilation stops: bash: /usr/bin/sort: cannot execute binary file Okey. The date on /usr/bin/sort is unchanged. Must be root to write. I am NOT compiling as root. "File" on /usr/bin/sort says "data". No elf. The only thing that the logs say: modprobe: Can't locate module binfmt-464c The filesystem on /usr is ext2. I downloaded a new textutils.deb and installed it. (But I made a backup of the corrupted file for some reason) Searched the net and found some previous problem with 2.2.10 and DMA + CompactFlash. Started to write a mail like this. Tried to do a ls -> Segmentation fault. Then the entire machine crashed. I rebooted the machine and fsck.ext2 complained about my largest partition, did a manual check and now I'm back. I wonder how many corrupted files there are left... The machine is usually very stable, but I haven't done much PCMCIA activity with it. Maybe this is not even related to PCMCIA... Anyway, this is what I do to my disk/controller: /sbin/hdparm -c1 -d1 -m16 -a0 -S4 -u1 /dev/hda I don't know if that has anything to do with it. I've used this computer about three months now and another Portege (3440CT) six months. Never experienced anything like that... So, all Portege users out there - be careful! Cheers / Magnus Software: linux-2.2.18 pcmcia-cs-3.1.23 Hardware: Toshiba Portege 3480CT: CPU and memory: 600MHz Speedstep PII 192MByte ram. Harddisk and controller: PCI_IDE: unknown IDE controller on PCI bus 00 device 39, VID=8086, DID=7199 PCI_IDE: not 100% native mode: will probe irqs later ide0: BM-DMA at 0xbff0-0xbff7, BIOS settings: hda:DMA, hdb:pio ide1: BM-DMA at 0xbff8-0xbfff, BIOS settings: hdc:pio, hdd:pio hda: TOSHIBA MK1214GAP, ATA DISK drive ide0 at 0x1f0-0x1f7,0x3f6 on irq 14 hda: TOSHIBA MK1214GAP, 11513MB w/0kB Cache, CHS=1467/255/63, UDMA Pcmcia/pccard: Linux PCMCIA Card Services 3.1.23 kernel build: 2.2.18 #2 Mon Jan 15 23:56:28 CET 2001 options: [pci] [cardbus] [apm] PCI routing table version 1.0 at 0xf0190 00:0b.0 -> irq 11 00:0b.1 -> irq 11 Intel PCIC probe: Toshiba ToPIC100 rev 20 PCI-to-CardBus at slot 00:0b, mem 0x6800 host opts [0]: [slot 0xf0] [ccr 0x16] [cdr 0x86] [rcr 0xc00] [pci irq 11] [lat 64/176] [bus 20/20] host opts [1]: [slot 0xf0] [ccr 0x26] [cdr 0x86] [rcr 0xc00] [pci irq 11] [lat 64/176] [bus 21/21] Compact Flash: hdc: Hitachi CV 7.2.2, ATA DISK drive hdc: Hitachi CV 7.2.2, 30MB w/1kB Cache, CHS=492/4/32 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] opl3sa2: MODULE_PARM_DESC
opl3sa2: Fix "irq"-parameter name typo for parameter description. Signed-off-by: Magnus Damm <[EMAIL PROTECTED]> --- linux-2.6.12-rc2/sound/oss/opl3sa2.c2005-04-05 16:57:30.0 +0200 +++ linux-2.6.12-rc2-autoparam/sound/oss/opl3sa2.c 2005-04-05 19:22:49.469074368 +0200 @@ -199,7 +199,7 @@ MODULE_PARM_DESC(mpu_io, "Set MIDI I/O base (0x330 or other. Address must be even and must be from 0x300 to 0x334)"); module_param(irq, int, 0); -MODULE_PARM_DESC(mss_irq, "Set MSS (audio) IRQ (5, 7, 9, 10, 11, 12)"); +MODULE_PARM_DESC(irq, "Set MSS (audio) IRQ (5, 7, 9, 10, 11, 12)"); module_param(dma, int, 0); MODULE_PARM_DESC(dma, "Set MSS (audio) first DMA channel (0, 1, 3)"); - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH][RFC] disable built-in modules V2
Here comes version 2 of the disable built-in patch.
This patch makes it possible to disable built-in code from the kernel
command line. The patch is rather simple - it extends the compiled-in case
of module_init() to include __setup() with a name based on KBUILD_MODNAME.
As an example, if you want to disable built-in firewire support then you
could pass "force_ohci1394=off" on the kernel command line. The feature added
by this patch comes handy when you want to boot a precompiled kernel from a
live cd on som exotic hardware that makes built-in drivers crash.
Changes since last release:
- Upgraded and tested with 2.6.12-rc2
- Added "force_"-prefix to cope with namespace pollution
Signed-off-by: Magnus Damm <[EMAIL PROTECTED]>
diff -urNp linux-2.6.12-rc2/include/linux/init.h
linux-2.6.12-rc2-disable_builtin/include/linux/init.h
--- linux-2.6.12-rc2/include/linux/init.h 2005-04-05 16:57:29.0
+0200
+++ linux-2.6.12-rc2-disable_builtin/include/linux/init.h 2005-04-06
00:26:07.0 +0200
@@ -143,6 +143,16 @@ struct obs_kernel_param {
/* Relies on saved_command_line being set */
void __init parse_early_param(void);
+
+void __init disable_initcall(void *fn);
+#define __module_init_disable(x) \
+static int __init x##_disable_module(char *str)\
+{ \
+ disable_initcall(x);\
+ return 1; \
+} \
+__setup("force_" __stringify(KBUILD_MODNAME) "=off", x##_disable_module)
+
#endif /* __ASSEMBLY__ */
/**
@@ -153,7 +163,7 @@ void __init parse_early_param(void);
* builtin) or at module insertion time (if a module). There can only
* be one per module.
*/
-#define module_init(x) __initcall(x);
+#define module_init(x) __initcall(x); __module_init_disable(x);
/**
* module_exit() - driver exit entry point
diff -urNp linux-2.6.12-rc2/init/main.c
linux-2.6.12-rc2-disable_builtin/init/main.c
--- linux-2.6.12-rc2/init/main.c2005-04-05 16:59:55.0 +0200
+++ linux-2.6.12-rc2-disable_builtin/init/main.c2005-04-05
21:07:05.0 +0200
@@ -539,6 +539,17 @@ struct task_struct *child_reaper = &init
extern initcall_t __initcall_start[], __initcall_end[];
+void __init disable_initcall(void *fn)
+{
+ initcall_t *call;
+
+ for (call = __initcall_start; call < __initcall_end; call++) {
+
+ if (*call == fn)
+ *call = NULL;
+ }
+}
+
static void __init do_initcalls(void)
{
initcall_t *call;
@@ -553,7 +564,8 @@ static void __init do_initcalls(void)
printk("\n");
}
- (*call)();
+ if (*call)
+ (*call)();
msg = NULL;
if (preempt_count() != count) {
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][RFC] disable built-in modules V2
On Apr 6, 2005 12:32 PM, Malcolm Rowe <[EMAIL PROTECTED]> wrote:
> Magnus Damm writes:
> > Here comes version 2 of the disable built-in patch.
>
> > +void __init disable_initcall(void *fn)
> > +{
> > + initcall_t *call;
> > +
> > + for (call = __initcall_start; call < __initcall_end; call++) {
> > +
> > + if (*call == fn)
> > + *call = NULL;
> > + }
> > +}
>
> Regardless of anything else, won't this break booting with initcall_debug on
> PPC64/IA64 machines? (see the definition of print_fn_descriptor_symbol() in
> kallsyms.h)
Correct, thanks for pointing that out. The code below is probably better:
static void __init do_initcalls(void)
{
initcall_t *call;
@@ -547,6 +558,9 @@ static void __init do_initcalls(void)
for (call = __initcall_start; call < __initcall_end; call++) {
char *msg;
+ if (!*call)
+ continue;
+
if (initcall_debug) {
printk(KERN_DEBUG "Calling initcall 0x%p", *call);
print_fn_descriptor_symbol(": %s()", (unsigned
long) *call);
And I guess the idea of replacing the initcall pointer with NULL will
work both with and without function descriptors, right? So we should
be safe on IA64 and PPC64.
Regards,
/ magnus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][RFC] disable built-in modules V2
On Apr 6, 2005 4:28 PM, Malcolm Rowe <[EMAIL PROTECTED]> wrote:
> Magnus Damm writes:
> > And I guess the idea of replacing the initcall pointer with NULL will
> > work both with and without function descriptors, right? So we should
> > be safe on IA64 and PPC64.
>
> I think so, though I don't really know a great deal about this area.
>
> An IA64 descriptor is of the form { &code, &data_context }, and a function
> pointer is a pointer to such a descriptor. Presumably, setting a function
> pointer to NULL will either end up setting the pointer-to-descriptor to NULL
> or the code pointer to NULL, but either way, I would expect the 'if
> (!*call)' comparison to work as intended.
>
> Best thing would be to get someone on IA64 and/or PPC64 to check this for
> you.
I agree. Do we have any friendly IA64/PPC64 user out there?
> Also might be worth checking that the patch works as intended with
> CONFIG_MODULES=n (assuming you haven't already).
The code works both with CONFIG_MODULES set to "y" and unset.
Thanks,
/ magnus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Kernel SCM saga..
On Apr 7, 2005 4:32 AM, David Lang <[EMAIL PROTECTED]> wrote: > On Thu, 7 Apr 2005, Martin Pool wrote: > > > I haven't tested importing all 60,000+ changesets of the current bk tree, > > partly because I don't *have* all those changesets. (Larry said > > previously that someone (not me) tried to pull all of them using bkclient, > > and he considered this abuse and blacklisted them.) > > pull the patches from the BK2CVS server. yes some patches are combined, > but it will get you in the ballpark. While at it, is there any ongoing effort to convert/export the kernel BK repository to some well known format like broken out patches and a series file? I think keeping the complete repository public in a well known format is important regardless of SCM taste. / magnus - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][RFC] disable built-in modules V2
On Apr 7, 2005 4:23 AM, Roland Dreier <[EMAIL PROTECTED]> wrote:
> > > -#define module_init(x) __initcall(x);
> > > +#define module_init(x) __initcall(x); __module_init_disable(x);
> >
> > It would be better if there is brackets around them... like
> >
> > #define module_init(x) { __initcall(x); __module_init_disable(x); }
> >
> > then we know it wont break some code like
> >
> > if (..)
> > module_init(x);
>
> This is all completely academic, since module_init() is a declaration
> that won't be inside any code, but in general it's better still to use
> the do { } while (0) idiom like
>
> #define module_init(x) do { __initcall(x); __module_init_disable(x); } while
> (0)
>
> so it won't break code like
>
> if (..)
> module_init(x);
> else
> something_else();
>
> (Yes, that code is nonsense but if you're going to nitpick, go all the way...)
Right. =)
Anyway, besides nitpicking, is there any reason not to include this
code? Or is the added feature considered plain bloat? Yes, the kernel
will become a bit larger, but all the data added by this patch will go
into the init section.
/ magnus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Kernel SCM saga..
On Apr 7, 2005 7:38 PM, Linus Torvalds <[EMAIL PROTECTED]> wrote: > So my prefernce is _overwhelmingly_ for the format that Andrew uses (which > is partly explained by the fact that I am used to it, but also by the fact > that I've asked for Andrew to make trivial changes to match my usage). > > That canonical format is: > > Subject: [PATCH 001/123] [:] > > together with the first line of the body being a > > From: Original Author <[EMAIL PROTECTED]> > > followed by an empty line and then the body of the explanation. > > After the body of the explanation comes the "Signed-off-by:" lines, and > then a simple "---" line, and below that comes the diffstat of the patch > and then the patch itself. While specifying things, wouldn't it be useful to have a line containing tags that specifies if the patch contains new features, a bug fix or a high-priority security fix? Then that information could be used to find patches for the sucker-tree. / magnus - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][RFC] disable built-in modules V2
On Apr 7, 2005 7:29 PM, Randy.Dunlap <[EMAIL PROTECTED]> wrote: > On Thu, 7 Apr 2005 13:22:57 -0400 (EDT) Richard B. Johnson wrote: > | Can't you disable module-loading with a module? I think so. > | You don't need to modify the kernel. Boot-scripts could > | just load the "final" module and there is nothing that > | can be done to add another module (or even unload existing > | ones). > > Sounds likely, but that's not what the patch from Magnus is > even trying to do. It's purely for boot-time selection of > troublesome modules or devices AFAICT. I've had a few > occasions to use something like this -- or rebuild a kernel > or remove a device. Yes, the idea is to be able to disable misbehaving built-in modules from the kernel command line. The alternative is of course to disable the code using the kernel configuration system and rebuild the kernel. Say a kernel shipped with your favourite distribution crashes your machine during boot-up - wouldn't it be nice to be able to just disable the problematic module from the kernel command line instead of (1) getting the config, (2) getting the distribution-specific patches, (3) rebuilding the kernel. And if you are lucky you need to (0) setup a cross compiler and (4) get your hands on a suitable initrd. / magnus - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH][RFC] disable built-in modules V2
On Apr 7, 2005 4:38 AM, Horst von Brand <[EMAIL PROTECTED]> wrote:
> AsterixTheGaul <[EMAIL PROTECTED]> said:
> > > -#define module_init(x) __initcall(x);
> > > +#define module_init(x) __initcall(x); __module_init_disable(x);
> >
> > It would be better if there is brackets around them... like
> >
> > #define module_init(x) { __initcall(x); __module_init_disable(x); }
> >
> > then we know it wont break some code like
> >
> > if (..)
> > module_init(x);
>
> But happily break:
>
>if (...)
> module_init(x);
>else
> ...
>
> This should be:
>
> #define module_init(x) do {__initcall(x); __module_init_disable(x);}while(0)
Yes and no. =) Wrapping defines in do {} while(0) is nice when you are
using the defined constants inside functions. module_init() OTOH is
never used inside a function and your suggestion leads to compile
errors:
CC arch/i386/kernel/dmi_scan.o
CC arch/i386/kernel/bootflag.o
arch/i386/kernel/bootflag.c:99: error: parse error before "do"
arch/i386/kernel/bootflag.c:99: error: parse error before '}' token
make[1]: *** [arch/i386/kernel/bootflag.o] Error 1
make: *** [arch/i386/kernel] Error 2
[EMAIL PROTECTED] linux-2.6.12-rc2-disable_builtin $
/ magnus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
