Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers
On Fri, Jul 06, 2012 at 08:08:23PM +, Arnd Bergmann wrote:
> On Thursday 05 July 2012, Andrew Lunn wrote:
> > > I think the latter one needs to be
> > >
> > > +static int __initdata gpio1_irqs[4] = {
> > > + IRQ_DOVE_HIGH_GPIO,
> > > + IRQ_DOVE_HIGH_GPIO,
> > > + IRQ_DOVE_HIGH_GPIO,
> > > + IRQ_DOVE_HIGH_GPIO,
> > > +};
> > >
> > > so we register all four parts to the same primary IRQ. The
> > > same is true for the devicetree representation.
> >
> > Nope, does not work like that.
> >
> > It does not matter which IRQ of a GPIO chip fires. It looks at the IRQ
> > cause bits for all lines and fires off the secondary ISR as needed for
> > the whole chip. So in effect, there is a mapping IRQ->GPIO chip, not
> > IRQ->1/4 of GPIO chip. With what you suggest above, you would end up
> > with four chained interrupt handlers, all being handled by the same
> > interrupt handler for one chio, and the last three in the chain would
> > never do anything since the first one does all the work.
>
> Does it really?
>
> The handler function I'm looking at is
>
>
> static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
> {
> int irqoff;
> BUG_ON(irq < IRQ_DOVE_GPIO_0_7 || irq > IRQ_DOVE_HIGH_GPIO);
>
> irqoff = irq <= IRQ_DOVE_GPIO_16_23 ? irq - IRQ_DOVE_GPIO_0_7 :
> 3 + irq - IRQ_DOVE_GPIO_24_31;
>
> orion_gpio_irq_handler(irqoff << 3);
> if (irq == IRQ_DOVE_HIGH_GPIO) {
> orion_gpio_irq_handler(40);
> orion_gpio_irq_handler(48);
> orion_gpio_irq_handler(56);
> }
> }
void orion_gpio_irq_handler(int pinoff)
{
struct orion_gpio_chip *ochip;
u32 cause, type;
int i;
ochip = orion_gpio_chip_find(pinoff);
if (ochip == NULL)
return;
cause = readl(GPIO_DATA_IN(ochip)) & readl(GPIO_LEVEL_MASK(ochip));
cause |= readl(GPIO_EDGE_CAUSE(ochip)) & readl(GPIO_EDGE_MASK(ochip));
for (i = 0; i < ochip->chip.ngpio; i++) {
int irq;
irq = ochip->secondary_irq_base + i;
if (!(cause & (1 << i)))
continue;
type = irqd_get_trigger_type(irq_get_irq_data(irq));
if ((type & IRQ_TYPE_SENSE_MASK) == IRQ_TYPE_EDGE_BOTH) {
/* Swap polarity (race with GPIO line) */
u32 polarity;
polarity = readl(GPIO_IN_POL(ochip));
polarity ^= 1 << i;
writel(polarity, GPIO_IN_POL(ochip));
}
generic_handle_irq(irq);
}
}
orion_gpio_chip_find(pinoff) when called with pinoff 32, 40, 48, or 56
will return the same gpio chip. The loop:
for (i = 0; i < ochip->chip.ngpio; i++) {
will iterate over all lines of the controller.
> My reading of this is a manual hardwired implementation of a
> primary handler that triggers the secondary handler four times
> when it's called with a specific argument.
Here is your mistake. It not a secondary handler. It is a function
which might trigger a secondary handler, if the status bit requires
that the secondary handler should be called..
> If you want to keep that behavior, this handler cannot be
> generic across all mvebu socs, whereas registering four
> chained handlers for the same primary interrupt would have
> the same effect at a very small runtime overhead without the
> need for any special case.
I would say the current code does redundant stuff.
This code has also been tested on a Dove by Sebastian Hesselbarth and
he did not notice anything strange happening on his system.
Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers
On Thursday 05 July 2012, Andrew Lunn wrote:
> > I think the latter one needs to be
> >
> > +static int __initdata gpio1_irqs[4] = {
> > + IRQ_DOVE_HIGH_GPIO,
> > + IRQ_DOVE_HIGH_GPIO,
> > + IRQ_DOVE_HIGH_GPIO,
> > + IRQ_DOVE_HIGH_GPIO,
> > +};
> >
> > so we register all four parts to the same primary IRQ. The
> > same is true for the devicetree representation.
>
> Nope, does not work like that.
>
> It does not matter which IRQ of a GPIO chip fires. It looks at the IRQ
> cause bits for all lines and fires off the secondary ISR as needed for
> the whole chip. So in effect, there is a mapping IRQ->GPIO chip, not
> IRQ->1/4 of GPIO chip. With what you suggest above, you would end up
> with four chained interrupt handlers, all being handled by the same
> interrupt handler for one chio, and the last three in the chain would
> never do anything since the first one does all the work.
Does it really?
The handler function I'm looking at is
static void gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
{
int irqoff;
BUG_ON(irq < IRQ_DOVE_GPIO_0_7 || irq > IRQ_DOVE_HIGH_GPIO);
irqoff = irq <= IRQ_DOVE_GPIO_16_23 ? irq - IRQ_DOVE_GPIO_0_7 :
3 + irq - IRQ_DOVE_GPIO_24_31;
orion_gpio_irq_handler(irqoff << 3);
if (irq == IRQ_DOVE_HIGH_GPIO) {
orion_gpio_irq_handler(40);
orion_gpio_irq_handler(48);
orion_gpio_irq_handler(56);
}
}
My reading of this is a manual hardwired implementation of a
primary handler that triggers the secondary handler four times
when it's called with a specific argument.
If you want to keep that behavior, this handler cannot be
generic across all mvebu socs, whereas registering four
chained handlers for the same primary interrupt would have
the same effect at a very small runtime overhead without the
need for any special case.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers
On 7/5/2012 4:54 AM, Arnd Bergmann wrote:
On Thursday 05 July 2012, Sebastian Hesselbarth wrote:
Andrew,
is it possible to group all gpio banks into one DT description?
For mach-dove it could be something like:
gpio: gpio-controller {
compatible = "marvell, orion-gpio";
...
bank0@d0400 {
reg = <0xd0400 0x40>;
ngpio = <8>;
mask-offset = <0>;
interrupts = <12>;
};
bank1@d0400 {
reg = <0xd0400 0x40>;
ngpio = <8>;
mask-offset = <8>;
interrupts = <13>;
};
This way you have multiple nodes with the same register
and different names, which is not how it normally works.
The "mask-offset" property is really a "reg" in disguise.
"reg" is considerably more general than just "memory mapped
register address". It really means "any numeric identifier
that makes sense in the context of a parent device".
Therefore, one could say:
gpio: gpio-controller {
compatible = "marvell, orion-gpio";
reg = <0xd0400 0x40>;
#address-cells = <1>;
#size-cells = <0>;
...
bank0@0 {
reg = <0x0>;
ngpio = <8>;
mask-offset = <0>;
interrupts = <12>;
};
bank1@8 {
reg = <0x8>;
ngpio = <8>;
interrupts = <13>;
};
This would have the advantage that DT describes gpio-to-irq dependencies.
Moreover, nodes that reference gpios can do gpios = <&gpio 71 0>; instead of
gpios = <&gpio3 7 0>;
Is that desired?
The device tree representation should match what is in the data sheet
normally. If they are in a single continuous number range, then we should
probably have a single device node with multiple register ranges
rather than one device node for each 32-bit register. Looking at
arch/arm/plat-orion/gpio.c I think that is not actually the case though
and having separate banks is more logical.
Something completely different I just noticed in the original patch:
@@ -90,6 +74,27 @@ static void pmu_irq_handler(unsigned int irq, struct
irq_desc *desc)
}
}
+static int __initdata gpio0_irqs[4] = {
+ IRQ_DOVE_GPIO_0_7,
+ IRQ_DOVE_GPIO_8_15,
+ IRQ_DOVE_GPIO_16_23,
+ IRQ_DOVE_GPIO_24_31,
+};
+
+static int __initdata gpio1_irqs[4] = {
+ IRQ_DOVE_HIGH_GPIO,
+ 0,
+ 0,
+ 0,
+};
I think the latter one needs to be
+static int __initdata gpio1_irqs[4] = {
+ IRQ_DOVE_HIGH_GPIO,
+ IRQ_DOVE_HIGH_GPIO,
+ IRQ_DOVE_HIGH_GPIO,
+ IRQ_DOVE_HIGH_GPIO,
+};
so we register all four parts to the same primary IRQ. The
same is true for the devicetree representation.
Arnd
___
devicetree-discuss mailing list
devicetree-disc...@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers
Sebastian Hesselbarth writes: > On 07/05/2012 04:54 PM, Arnd Bergmann wrote: >> This way you have multiple nodes with the same register >> and different names, which is not how it normally works. > > Ok. > >>> This would have the advantage that DT describes gpio-to-irq dependencies. >>> Moreover, nodes that reference gpios can do gpios =<&gpio 71 0>; instead of >>> gpios =<&gpio3 7 0>; >> >> Is that desired? >> >> The device tree representation should match what is in the data sheet >> normally. If they are in a single continuous number range, then we should >> probably have a single device node with multiple register ranges >> rather than one device node for each 32-bit register. Looking at >> arch/arm/plat-orion/gpio.c I think that is not actually the case though >> and having separate banks is more logical. > > Well, looking at the datasheet of Dove GPIOs are numbered [63:0] plus > GPOs [71:64]. This dt will be a lot shorter and maybe it is describing > the hardware as it is. (Not sure about the syntax for irqs, though) They're numbered as [63:0] and [71:64] but they're on 3 different banks. iirc, there may even be some differences with the way the banks are dealing interrupts, so I don't see any reason to not represent the 3 banks in DT. Arnaud -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers
> Something completely different I just noticed in the original patch:
>
> > @@ -90,6 +74,27 @@ static void pmu_irq_handler(unsigned int irq, struct
> > irq_desc *desc)
> > }
> > }
> >
> > +static int __initdata gpio0_irqs[4] = {
> > + IRQ_DOVE_GPIO_0_7,
> > + IRQ_DOVE_GPIO_8_15,
> > + IRQ_DOVE_GPIO_16_23,
> > + IRQ_DOVE_GPIO_24_31,
> > +};
> > +
> > +static int __initdata gpio1_irqs[4] = {
> > + IRQ_DOVE_HIGH_GPIO,
> > + 0,
> > + 0,
> > + 0,
> > +};
>
> I think the latter one needs to be
>
> +static int __initdata gpio1_irqs[4] = {
> + IRQ_DOVE_HIGH_GPIO,
> + IRQ_DOVE_HIGH_GPIO,
> + IRQ_DOVE_HIGH_GPIO,
> + IRQ_DOVE_HIGH_GPIO,
> +};
>
> so we register all four parts to the same primary IRQ. The
> same is true for the devicetree representation.
Nope, does not work like that.
It does not matter which IRQ of a GPIO chip fires. It looks at the IRQ
cause bits for all lines and fires off the secondary ISR as needed for
the whole chip. So in effect, there is a mapping IRQ->GPIO chip, not
IRQ->1/4 of GPIO chip. With what you suggest above, you would end up
with four chained interrupt handlers, all being handled by the same
interrupt handler for one chio, and the last three in the chain would
never do anything since the first one does all the work.
Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers
On 07/05/2012 04:54 PM, Arnd Bergmann wrote:
This way you have multiple nodes with the same register
and different names, which is not how it normally works.
Ok.
This would have the advantage that DT describes gpio-to-irq dependencies.
Moreover, nodes that reference gpios can do gpios =<&gpio 71 0>; instead of
gpios =<&gpio3 7 0>;
Is that desired?
The device tree representation should match what is in the data sheet
normally. If they are in a single continuous number range, then we should
probably have a single device node with multiple register ranges
rather than one device node for each 32-bit register. Looking at
arch/arm/plat-orion/gpio.c I think that is not actually the case though
and having separate banks is more logical.
Well, looking at the datasheet of Dove GPIOs are numbered [63:0] plus
GPOs [71:64]. This dt will be a lot shorter and maybe it is describing
the hardware as it is. (Not sure about the syntax for irqs, though)
gpio@d0400 {
compatible = "marvell,orion-gpio";
gpio-controller;
reg = <0xd0400 0x20>, /* GPIO[31: 0] */
<0xd0420 0x20>, /* GPIO[63:32] */
<0xe8400 0x0c>; /* GPO [71:64] */
ngpio = <72>;
interrupts = <12 13 14 15>, <61>;
};
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers
On Thursday 05 July 2012, Sebastian Hesselbarth wrote:
> Andrew,
>
> is it possible to group all gpio banks into one DT description?
> For mach-dove it could be something like:
>
> gpio: gpio-controller {
> compatible = "marvell, orion-gpio";
> ...
>
> bank0@d0400 {
> reg = <0xd0400 0x40>;
> ngpio = <8>;
> mask-offset = <0>;
> interrupts = <12>;
> };
>
> bank1@d0400 {
> reg = <0xd0400 0x40>;
> ngpio = <8>;
> mask-offset = <8>;
> interrupts = <13>;
> };
This way you have multiple nodes with the same register
and different names, which is not how it normally works.
>
> This would have the advantage that DT describes gpio-to-irq dependencies.
> Moreover, nodes that reference gpios can do gpios = <&gpio 71 0>; instead of
> gpios = <&gpio3 7 0>;
Is that desired?
The device tree representation should match what is in the data sheet
normally. If they are in a single continuous number range, then we should
probably have a single device node with multiple register ranges
rather than one device node for each 32-bit register. Looking at
arch/arm/plat-orion/gpio.c I think that is not actually the case though
and having separate banks is more logical.
Something completely different I just noticed in the original patch:
> @@ -90,6 +74,27 @@ static void pmu_irq_handler(unsigned int irq, struct
> irq_desc *desc)
> }
> }
>
> +static int __initdata gpio0_irqs[4] = {
> + IRQ_DOVE_GPIO_0_7,
> + IRQ_DOVE_GPIO_8_15,
> + IRQ_DOVE_GPIO_16_23,
> + IRQ_DOVE_GPIO_24_31,
> +};
> +
> +static int __initdata gpio1_irqs[4] = {
> + IRQ_DOVE_HIGH_GPIO,
> + 0,
> + 0,
> + 0,
> +};
I think the latter one needs to be
+static int __initdata gpio1_irqs[4] = {
+ IRQ_DOVE_HIGH_GPIO,
+ IRQ_DOVE_HIGH_GPIO,
+ IRQ_DOVE_HIGH_GPIO,
+ IRQ_DOVE_HIGH_GPIO,
+};
so we register all four parts to the same primary IRQ. The
same is true for the devicetree representation.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers
On Thu, Jul 05, 2012 at 04:14:50PM +0200, Sebastian Hesselbarth wrote:
> On 07/05/2012 03:08 PM, Andrew Lunn wrote:
> >The issue is knowing what IRQ number to use for the secondary
> >interrupts.
> >
> >Orion use generic chip interrupts, both for the main interrupts and
> >the GPIO interrupts. This does not yet support irq domain, so i have
> >to layer a legacy domain on top. The legacy domain needs to know the
> >first IRQ and the number of IRQs. For the primary IRQs that is
> >easy. However, GPIO IRQ is not so easy, it depends on how many primary
> >IRQs there are. This is not fixed. Orion5x has 32, Dove 64, kirkwood,
> >64, and mv78xx0 has 96. I need to know this number when adding the
> >GPIO secondary IRQ legacy domain. By calling orion_gpio_of_init() in
> >the orion_add_irq_domain() i have this number to hand. If i used to
> >entries in the match table, i would have to put this number into some
> >global variable, or somehow ask the IRQ subsystem what the next free
> >IRQ number is.
>
> Andrew,
>
> is it possible to group all gpio banks into one DT description?
Everything is possible.
I did think about having just one gpio controller, since as you said,
it makes it easier to describe gpios = <&gpio 71 0>; instead of gpios
= <&gpio3 7 0>; But most SoCs seem to have multiple GPIO controllers
in the .dtsi files. Only picoxcell has banks inside a single gpio
controller. So it would be a bit unusual.
What you suggest also adds a lot of complexity to the code and
probably means a lot less of the code can be shared with non-DT GPIO
handling code.
The natural size of a GPIO controller is 32 lines, not 8. The current
interrupt handling does not actually care which IRQ of a bank of 32
lines fired, it looks at all lines. The handler_data in the IRQ points
to the GPIO chip, not a subsection of the GPIO chip.
> bank5@e8400 {
> reg = <0xe8400 0x20>;
> ngpio = <8>;
> marvell,orion-gpio-output-only;
Some Orions mixed GPIOs GPOs and GPIs within one controller. So that
is not generic enough. Probably the pinmux DT description needs to be
able to specify per pin what a line can do.
Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers
On 07/05/2012 03:08 PM, Andrew Lunn wrote:
The issue is knowing what IRQ number to use for the secondary
interrupts.
Orion use generic chip interrupts, both for the main interrupts and
the GPIO interrupts. This does not yet support irq domain, so i have
to layer a legacy domain on top. The legacy domain needs to know the
first IRQ and the number of IRQs. For the primary IRQs that is
easy. However, GPIO IRQ is not so easy, it depends on how many primary
IRQs there are. This is not fixed. Orion5x has 32, Dove 64, kirkwood,
64, and mv78xx0 has 96. I need to know this number when adding the
GPIO secondary IRQ legacy domain. By calling orion_gpio_of_init() in
the orion_add_irq_domain() i have this number to hand. If i used to
entries in the match table, i would have to put this number into some
global variable, or somehow ask the IRQ subsystem what the next free
IRQ number is.
Andrew,
is it possible to group all gpio banks into one DT description?
For mach-dove it could be something like:
gpio: gpio-controller {
compatible = "marvell, orion-gpio";
...
bank0@d0400 {
reg = <0xd0400 0x40>;
ngpio = <8>;
mask-offset = <0>;
interrupts = <12>;
};
bank1@d0400 {
reg = <0xd0400 0x40>;
ngpio = <8>;
mask-offset = <8>;
interrupts = <13>;
};
...
bank4@d0420 {
reg = <0xd0420 0x40>;
ngpio = <32>;
interrupts = <61>;
};
bank5@e8400 {
reg = <0xe8400 0x20>;
ngpio = <8>;
marvell,orion-gpio-output-only;
};
};
This would have the advantage that DT describes gpio-to-irq dependencies.
Moreover, nodes that reference gpios can do gpios = <&gpio 71 0>; instead of
gpios = <&gpio3 7 0>;
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers
On Thursday 05 July 2012, Andrew Lunn wrote: > > > > I'm wondering about this one. The other platforms usually put the secondary > > interrupt controllers into the same match table, while you call > > orion_gpio_of_init > > from orion_add_irq_domain. Can you explain why you do this? Does it have > > any disadvantages? > > The issue is knowing what IRQ number to use for the secondary > interrupts. > > Orion use generic chip interrupts, both for the main interrupts and > the GPIO interrupts. This does not yet support irq domain, so i have > to layer a legacy domain on top. The legacy domain needs to know the > first IRQ and the number of IRQs. For the primary IRQs that is > easy. However, GPIO IRQ is not so easy, it depends on how many primary > IRQs there are. This is not fixed. Orion5x has 32, Dove 64, kirkwood, > 64, and mv78xx0 has 96. I need to know this number when adding the > GPIO secondary IRQ legacy domain. By calling orion_gpio_of_init() in > the orion_add_irq_domain() i have this number to hand. If i used to > entries in the match table, i would have to put this number into some > global variable, or somehow ask the IRQ subsystem what the next free > IRQ number is. But couldn't you store the number of interrupts for the primary controller in a local variable? I think the of_irq code already guarantees that the parent is probed first. > As for disadvantages, humm. Dove has yet more interrupts, from the > PMU. They are currently unsupported in DT. When we add support for the > PMU interrupt controller, we are going to have the same problem, what > IRQ base should it use. Either we extend the chaining, calling > dove_pmu_of_init from orion_gpio_of_init(), where we know the next > free IRQ. Or we find out how to ask the IRQ subsystem for the next > available. Better still, the work to make generic chip interrupts irq > domain aware would get completed, and we can swap all this code to irq > domain linear and this whole probable probably goes away. Yes, that makes sense. Using the linear domain should solve all these nicely. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers
Le Thu, 5 Jul 2012 15:15:22 +0200, Andrew Lunn a écrit : > > > The biggest problem i had, is the interaction between generic chip > > > interrupts and irqdomain. There has been work to integrate the two, > > > but its stalled. Either the work needs restarting and completing, or > > > you need to throw away the use of generic interrupt so that you can > > > use irqdomain linear. IMHO, throwing away generic interrupt is the > > > wrong way. > > > > Can you expand on why you think it would be wrong to throw away the > > usage of irq_chip_generic, compared to implementing directly an > > irq_chip? > > Basically you are asking, why should i use the framework when i can do > it by hand. Agreed :) > What are the advantages if ignoring the framework and doing it by > hand? Many GPIO drivers directly use irq_chip (though it's true some of them use irq_chip_generic), and the irq_chip_generic framework doesn't have really proper support for the other new irqdomain framework, so I was wondering what the best solution was. I found the proposal from Rob Herring to improve that situation, but apparently the conclusion from Grant Likely was "Let's discuss this at the next Connect". That patch set was posted in January, so two Linaro Connects happened since them, I don't know if progress has been made here. I guess I'll just stick to irq_chip_generic + irqdomain_legacy for now. Best regards, Thomas -- Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers
On Thu, Jul 05, 2012 at 02:58:01PM +0200, Thomas Petazzoni wrote: > Hello, > > Le Thu, 5 Jul 2012 11:48:24 +0200, > Andrew Lunn a ??crit : > > > The biggest problem i had, is the interaction between generic chip > > interrupts and irqdomain. There has been work to integrate the two, > > but its stalled. Either the work needs restarting and completing, or > > you need to throw away the use of generic interrupt so that you can > > use irqdomain linear. IMHO, throwing away generic interrupt is the > > wrong way. > > Can you expand on why you think it would be wrong to throw away the > usage of irq_chip_generic, compared to implementing directly an > irq_chip? Basically you are asking, why should i use the framework when i can do it by hand. What are the advantages if ignoring the framework and doing it by hand? Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers
> > +Required properties
> > +- compatible : Should be "marvell,orion-intc".
> > +- #interrupt-cells: Specifies the number of cells needed to encode an
> > + interrupt source. Supported value is <1>.
> > +- interrupt-controller : Declare this node to be an interrupt controller.
> > +- reg : Interrupt mask address.
>
> I think you should clarify that the "reg" property can be multiple
> 4-byte ranges, because that is fairly unusual.
O.K. The example is already like this, but i can add some more words.
> > +#ifdef CONFIG_OF
> > +static int __init orion_add_irq_domain(struct device_node *np,
> > + struct device_node *interrupt_parent)
> > +{
> > + int i = 0, irq_gpio;
> > + void __iomem *base;
> > +
> > + do {
> > + base = of_iomap(np, i);
> > + if (base) {
> > + orion_irq_init(i * 32, base);
> > + i++;
> > + }
> > + } while (base);
> > +
> > + irq_domain_add_legacy(np, i * 32, 0, 0,
> > + &irq_domain_simple_ops, NULL);
> > +
> > + irq_gpio = i * 32;
> > + orion_gpio_of_init(irq_gpio);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id orion_irq_match[] = {
> > + { .compatible = "marvell,orion-intc",
> > + .data = orion_add_irq_domain, },
> > + {},
> > +};
> > +
> > +void __init orion_dt_init_irq(void)
> > +{
> > + of_irq_init(orion_irq_match);
> > +}
> > +#endif
>
> I'm wondering about this one. The other platforms usually put the secondary
> interrupt controllers into the same match table, while you call
> orion_gpio_of_init
> from orion_add_irq_domain. Can you explain why you do this? Does it have
> any disadvantages?
The issue is knowing what IRQ number to use for the secondary
interrupts.
Orion use generic chip interrupts, both for the main interrupts and
the GPIO interrupts. This does not yet support irq domain, so i have
to layer a legacy domain on top. The legacy domain needs to know the
first IRQ and the number of IRQs. For the primary IRQs that is
easy. However, GPIO IRQ is not so easy, it depends on how many primary
IRQs there are. This is not fixed. Orion5x has 32, Dove 64, kirkwood,
64, and mv78xx0 has 96. I need to know this number when adding the
GPIO secondary IRQ legacy domain. By calling orion_gpio_of_init() in
the orion_add_irq_domain() i have this number to hand. If i used to
entries in the match table, i would have to put this number into some
global variable, or somehow ask the IRQ subsystem what the next free
IRQ number is.
As for disadvantages, humm. Dove has yet more interrupts, from the
PMU. They are currently unsupported in DT. When we add support for the
PMU interrupt controller, we are going to have the same problem, what
IRQ base should it use. Either we extend the chaining, calling
dove_pmu_of_init from orion_gpio_of_init(), where we know the next
free IRQ. Or we find out how to ask the IRQ subsystem for the next
available. Better still, the work to make generic chip interrupts irq
domain aware would get completed, and we can swap all this code to irq
domain linear and this whole probable probably goes away.
Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers
Hello, Le Thu, 5 Jul 2012 11:48:24 +0200, Andrew Lunn a écrit : > The biggest problem i had, is the interaction between generic chip > interrupts and irqdomain. There has been work to integrate the two, > but its stalled. Either the work needs restarting and completing, or > you need to throw away the use of generic interrupt so that you can > use irqdomain linear. IMHO, throwing away generic interrupt is the > wrong way. Can you expand on why you think it would be wrong to throw away the usage of irq_chip_generic, compared to implementing directly an irq_chip? Thanks! Thomas -- Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers
On Tuesday 03 July 2012, Andrew Lunn wrote:
> Both IRQ and GPIO controllers can now be represented in DT. The IRQ
> controllers are setup first, and then the GPIO controllers. Interrupts
> for GPIO lines are placed directly after the main interrupts in the
> interrupt space.
Overall looks very good.
> diff --git a/Documentation/devicetree/bindings/arm/mrvl/intc.txt
> b/Documentation/devicetree/bindings/arm/mrvl/intc.txt
> index 80b9a94..8927e10 100644
> --- a/Documentation/devicetree/bindings/arm/mrvl/intc.txt
> +++ b/Documentation/devicetree/bindings/arm/mrvl/intc.txt
> @@ -38,3 +38,22 @@ Example:
> reg-names = "mux status", "mux mask";
> mrvl,intc-nr-irqs = <2>;
> };
> +
> +* Marvell Orion Interrupt controller
> +
> +Required properties
> +- compatible : Should be "marvell,orion-intc".
> +- #interrupt-cells: Specifies the number of cells needed to encode an
> + interrupt source. Supported value is <1>.
> +- interrupt-controller : Declare this node to be an interrupt controller.
> +- reg : Interrupt mask address.
I think you should clarify that the "reg" property can be multiple
4-byte ranges, because that is fairly unusual.
> +#ifdef CONFIG_OF
> +static int __init orion_add_irq_domain(struct device_node *np,
> +struct device_node *interrupt_parent)
> +{
> + int i = 0, irq_gpio;
> + void __iomem *base;
> +
> + do {
> + base = of_iomap(np, i);
> + if (base) {
> + orion_irq_init(i * 32, base);
> + i++;
> + }
> + } while (base);
> +
> + irq_domain_add_legacy(np, i * 32, 0, 0,
> + &irq_domain_simple_ops, NULL);
> +
> + irq_gpio = i * 32;
> + orion_gpio_of_init(irq_gpio);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id orion_irq_match[] = {
> + { .compatible = "marvell,orion-intc",
> + .data = orion_add_irq_domain, },
> + {},
> +};
> +
> +void __init orion_dt_init_irq(void)
> +{
> + of_irq_init(orion_irq_match);
> +}
> +#endif
I'm wondering about this one. The other platforms usually put the secondary
interrupt controllers into the same match table, while you call
orion_gpio_of_init
from orion_add_irq_domain. Can you explain why you do this? Does it have
any disadvantages?
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers
On 07/05/2012 01:48 PM, Andrew Lunn wrote: >> Yes. I am not sure yet how to describe those in the DT, or even if it >> is actually useful to describe them. Wouldn't it be simpler to just >> leave to the user of the GPIO to use a GPIO that's appropriate for its >> usage, i.e not use a GPO when input is needed? > > We assume the hardware designer has got the basic hardware right. Its > not going to work otherwise. > > What we are trying to detect is a DT author making a typo, assigning a > gpio-key to a GPO pin, for example. The current MPP scheme will detect > this sort of error and issue a warning to the kernel logs. The output-only/non-interrupt gpios could be marked as such in DT. The common code should not install irqs if marked by e.g. "marvell,gpio-no-irq". The gpio driver already fails to set input direction on output-only pins. This could be marked by e.g. "marvell,gpio-output-only" in DT. Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers
Hi Thomas > Yes. I am not sure yet how to describe those in the DT, or even if it > is actually useful to describe them. Wouldn't it be simpler to just > leave to the user of the GPIO to use a GPIO that's appropriate for its > usage, i.e not use a GPO when input is needed? We assume the hardware designer has got the basic hardware right. Its not going to work otherwise. What we are trying to detect is a DT author making a typo, assigning a gpio-key to a GPO pin, for example. The current MPP scheme will detect this sort of error and issue a warning to the kernel logs. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers
Le Thu, 05 Jul 2012 12:38:51 +0200, Arnaud Patard (Rtp) a écrit : > > The MPP registers are identical on Armada XP/370 and 88F6281 (not sure > > which other SoC datasheet I should be checking). Basically, it's just a > > range of contiguous registers, with 4 bits per pin to select the > > function. > > iirc, other SoCs are similar. The small exception being dove I > guess. Dove has a 3rd gpo [the gpios are output only] bank but to be > used as gpio require that a special bit is set and it's for all gpios of > this bank. You'll find this bit in the "general mpp configuration > register" if you look at the datasheet. Ok, this is a bit trickier, but we can probably do something for it as well. > > So my pinmux driver should simply work as is for Orion as well. The > > only difference between platforms is the number of MPP pins that are > > available, but this number also varies between versions of Armada XP > > and Armada 370, so I already support this in the driver. > > Are there some output-only gpio on armada xp/370 like on kirkwood/dove ? Yes. I am not sure yet how to describe those in the DT, or even if it is actually useful to describe them. Wouldn't it be simpler to just leave to the user of the GPIO to use a GPIO that's appropriate for its usage, i.e not use a GPO when input is needed? Best regards, Thomas -- Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers
Thomas Petazzoni writes: > Le Thu, 05 Jul 2012 12:11:40 +0200, > Arnaud Patard (Rtp) a écrit : > >> > You are not the only one working in this area. Arnaud Patard said he >> > was look at this as well. >> >> yeah, but tbh I've not made anything yet. If Thomas has already some >> code for it, we should try to make it "generic" so as to use it on >> armada xp and orion platforms. > > The MPP registers are identical on Armada XP/370 and 88F6281 (not sure > which other SoC datasheet I should be checking). Basically, it's just a > range of contiguous registers, with 4 bits per pin to select the > function. iirc, other SoCs are similar. The small exception being dove I guess. Dove has a 3rd gpo [the gpios are output only] bank but to be used as gpio require that a special bit is set and it's for all gpios of this bank. You'll find this bit in the "general mpp configuration register" if you look at the datasheet. > > So my pinmux driver should simply work as is for Orion as well. The > only difference between platforms is the number of MPP pins that are > available, but this number also varies between versions of Armada XP > and Armada 370, so I already support this in the driver. > Are there some output-only gpio on armada xp/370 like on kirkwood/dove ? Arnaud -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers
> I've looked at the datasheet for the 88F6281, and the GPIO registers > look fairly similar with Armada 370/XP, except that: > > * On 88F6281, there are two GPIO banks, while on Armada 370/XP it's a >continuous range of registers that control the GPIOs. But it can be >assimilated as one bank handling more than 32 GPIOs. So shouldn't be >a big problem to handle. The number of banks varies. Orion5x has one, Dove has three, etc. > > * The Armada 370/XP have additional set/clear registers that allow to >modify the GPIO states without having to do a read/mask/write >sequence, which is nicer on SMP. I guess we can handle that as a >variant inside the driver without too much problem. > > Marvell has given me a 88F6282 eval board, so I guess they are fine > with me spending some time ensuring that the new code works properly on > the previous platforms, especially in this case where the amount of > work does not seem to be enormous to keep the compatibility. That is good to hear. Thanks Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers
Andrew Lunn writes: Hi, > On Thu, Jul 05, 2012 at 11:02:51AM +0200, Thomas Petazzoni wrote: >> Hello Andrew, >> >> Le Tue, 3 Jul 2012 16:22:34 +0200, >> Andrew Lunn a ??crit : >> >> > Both IRQ and GPIO controllers can now be represented in DT. The IRQ >> > controllers are setup first, and then the GPIO controllers. Interrupts >> > for GPIO lines are placed directly after the main interrupts in the >> > interrupt space. >> > >> > Signed-off-by: Andrew Lunn >> >> I have started working on a pinctrl driver for mvebu, which would >> handle pin muxing (MPP) + gpio + gpio interrupts. > > Hi Thomas > > You are not the only one working in this area. Arnaud Patard said he > was look at this as well. yeah, but tbh I've not made anything yet. If Thomas has already some code for it, we should try to make it "generic" so as to use it on armada xp and orion platforms. > >> So far, the pin muxing part is working (needs some polishing, but the >> foundation is here), with device tree bindings. I think the pin muxing >> could be used for Orion as well. > > Great. This is one of the big things which we are missing when moving > a system over the DT. Being able to describe this in DT in a > standardized way is very much welcome. > >> Now, I'm planning to start working on the gpio + gpio interrupts parts >> of the driver, and I'm wondering how to interact with your work on the >> matter. > > I've publicly said, what i have is enough to get it working, but i > know its not the end solution. What i have is enough that gpio-key, > gpio-led, as described in DT, works. Boards can start using these and > as far as i can tell, the binding should not need to change. The GPIO > controller binding should also be sufficiently generic that it should > also not need changes when we replace the driver with pinctrl. > > The biggest problem i had, is the interaction between generic chip > interrupts and irqdomain. There has been work to integrate the two, > but its stalled. Either the work needs restarting and completing, or > you need to throw away the use of generic interrupt so that you can > use irqdomain linear. IMHO, throwing away generic interrupt is the > wrong way. > > The other thing you need to keep in mind is the namespace issues > between normal interrupts and GPIO interrupts. The way my patch works > is that normal interrupts are setup first, counting how many have been > created. Then GPIO interrupts are added, starting off where the normal > interrupts ended. Since 370/XP uses different interrupt code, you need > to think how to solve this issue. > >> My understanding is that the new way of doing a pinmux+gpio >> driver is to implement it in drivers/pinctrl/, which I have started >> doing. > > Great. > >> Should I continue working on a drivers/pinctrl/ driver for mvebu for >> Armada 370/XP, and then we see together if it makes sense to extend to >> Orion, and if so, what changes are needed? > > I know Marvell has contracted you to port to Armada 370/XP, not mvebu, > aka all chips which might be able to share this code. This IMHO is > wrong. So i personally would put the kirkwood and the Armada 370/XP > data sheets next to each other, and from the beginning on write the > driver so it supports them all. This assumes the ASIC engineers have > not radically changed anything... Please also try to stick to the DT > binding i've used. agreed. it should be written with all platforms in mind, as long as they're compatible. It can only make things better imho. Arnaud -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers
Le Thu, 05 Jul 2012 12:11:40 +0200, Arnaud Patard (Rtp) a écrit : > > You are not the only one working in this area. Arnaud Patard said he > > was look at this as well. > > yeah, but tbh I've not made anything yet. If Thomas has already some > code for it, we should try to make it "generic" so as to use it on > armada xp and orion platforms. The MPP registers are identical on Armada XP/370 and 88F6281 (not sure which other SoC datasheet I should be checking). Basically, it's just a range of contiguous registers, with 4 bits per pin to select the function. So my pinmux driver should simply work as is for Orion as well. The only difference between platforms is the number of MPP pins that are available, but this number also varies between versions of Armada XP and Armada 370, so I already support this in the driver. However, using the pinmux driver will require changes in the device drivers, so that they request the pinmux for their devices. > > I know Marvell has contracted you to port to Armada 370/XP, not mvebu, > > aka all chips which might be able to share this code. This IMHO is > > wrong. So i personally would put the kirkwood and the Armada 370/XP > > data sheets next to each other, and from the beginning on write the > > driver so it supports them all. This assumes the ASIC engineers have > > not radically changed anything... Please also try to stick to the DT > > binding i've used. > > agreed. it should be written with all platforms in mind, as long as > they're compatible. It can only make things better imho. Certainly, especially for MPP/GPIO that are definitely very similar. Best regards, Thomas -- Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers
Hello,
Le Thu, 5 Jul 2012 11:48:24 +0200,
Andrew Lunn a écrit :
> Hi Thomas
>
> You are not the only one working in this area. Arnaud Patard said he
> was look at this as well.
Ok, I added Arnaud in the Cc list. Arnaud, what is the status of you
work?
> > So far, the pin muxing part is working (needs some polishing, but the
> > foundation is here), with device tree bindings. I think the pin muxing
> > could be used for Orion as well.
>
> Great. This is one of the big things which we are missing when moving
> a system over the DT. Being able to describe this in DT in a
> standardized way is very much welcome.
For the DT binding, I took my inspiration from the MXS and Samsung
pinctrl drivers, as those platforms also have independent pin muxing
(while some platforms such as Tegra, have hardware pin groups).
The .dtsi side looks like:
pinctrl@d0018000 {
compatible = "marvell,mv78230-pinmux";
reg = <0xd0018000 0x38>;
uart_2: uart-2 {
marvell,pins = <42 43>;
marvell,pins-function = <1>;
};
uart_3: uart-3 {
marvell,pins = <44 45>;
marvell,pins-function = <2>;
};
uart_0_control: uart-0-control {
marvell,pins = <42 43>;
marvell,pins-function = <2>;
};
uart_1_control: uart-1-control {
marvell,pins = <46 47>;
marvell,pins-function = <2>;
};
uart_2_control: uart-2-control {
marvell,pins = <44 45>;
marvell,pins-function = <1>;
};
uart_3_control: uart-3-control {
marvell,pins = <46 47>;
marvell,pins-function = <1>;
};
};
This defines the various pin muxing options, and then when
instantiating a device, you do:
test@12 {
compatible = "marvell,testdrv";
pinctrl-names = "default";
pinctrl-0 = <&uart_0_control>;
};
(This is with a dummy test driver that I'm using for testing). Then
the driver can use the pinctrl API to get the proper muxing for its
pins.
> > Now, I'm planning to start working on the gpio + gpio interrupts parts
> > of the driver, and I'm wondering how to interact with your work on the
> > matter.
>
> I've publicly said, what i have is enough to get it working, but i
> know its not the end solution. What i have is enough that gpio-key,
> gpio-led, as described in DT, works. Boards can start using these and
> as far as i can tell, the binding should not need to change. The GPIO
> controller binding should also be sufficiently generic that it should
> also not need changes when we replace the driver with pinctrl.
>
> The biggest problem i had, is the interaction between generic chip
> interrupts and irqdomain. There has been work to integrate the two,
> but its stalled. Either the work needs restarting and completing, or
> you need to throw away the use of generic interrupt so that you can
> use irqdomain linear. IMHO, throwing away generic interrupt is the
> wrong way.
>
> The other thing you need to keep in mind is the namespace issues
> between normal interrupts and GPIO interrupts. The way my patch works
> is that normal interrupts are setup first, counting how many have been
> created. Then GPIO interrupts are added, starting off where the normal
> interrupts ended. Since 370/XP uses different interrupt code, you need
> to think how to solve this issue.
Ok, thanks for those hints.
> > Should I continue working on a drivers/pinctrl/ driver for mvebu for
> > Armada 370/XP, and then we see together if it makes sense to extend to
> > Orion, and if so, what changes are needed?
>
> I know Marvell has contracted you to port to Armada 370/XP, not mvebu,
> aka all chips which might be able to share this code. This IMHO is
> wrong. So i personally would put the kirkwood and the Armada 370/XP
> data sheets next to each other, and from the beginning on write the
> driver so it supports them all. This assumes the ASIC engineers have
> not radically changed anything... Please also try to stick to the DT
> binding i've used.
I've looked at the datasheet for the 88F6281, and the GPIO registers
look fairly similar with A
Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers
On Thu, Jul 05, 2012 at 11:02:51AM +0200, Thomas Petazzoni wrote: > Hello Andrew, > > Le Tue, 3 Jul 2012 16:22:34 +0200, > Andrew Lunn a ??crit : > > > Both IRQ and GPIO controllers can now be represented in DT. The IRQ > > controllers are setup first, and then the GPIO controllers. Interrupts > > for GPIO lines are placed directly after the main interrupts in the > > interrupt space. > > > > Signed-off-by: Andrew Lunn > > I have started working on a pinctrl driver for mvebu, which would > handle pin muxing (MPP) + gpio + gpio interrupts. Hi Thomas You are not the only one working in this area. Arnaud Patard said he was look at this as well. > So far, the pin muxing part is working (needs some polishing, but the > foundation is here), with device tree bindings. I think the pin muxing > could be used for Orion as well. Great. This is one of the big things which we are missing when moving a system over the DT. Being able to describe this in DT in a standardized way is very much welcome. > Now, I'm planning to start working on the gpio + gpio interrupts parts > of the driver, and I'm wondering how to interact with your work on the > matter. I've publicly said, what i have is enough to get it working, but i know its not the end solution. What i have is enough that gpio-key, gpio-led, as described in DT, works. Boards can start using these and as far as i can tell, the binding should not need to change. The GPIO controller binding should also be sufficiently generic that it should also not need changes when we replace the driver with pinctrl. The biggest problem i had, is the interaction between generic chip interrupts and irqdomain. There has been work to integrate the two, but its stalled. Either the work needs restarting and completing, or you need to throw away the use of generic interrupt so that you can use irqdomain linear. IMHO, throwing away generic interrupt is the wrong way. The other thing you need to keep in mind is the namespace issues between normal interrupts and GPIO interrupts. The way my patch works is that normal interrupts are setup first, counting how many have been created. Then GPIO interrupts are added, starting off where the normal interrupts ended. Since 370/XP uses different interrupt code, you need to think how to solve this issue. > My understanding is that the new way of doing a pinmux+gpio > driver is to implement it in drivers/pinctrl/, which I have started > doing. Great. > Should I continue working on a drivers/pinctrl/ driver for mvebu for > Armada 370/XP, and then we see together if it makes sense to extend to > Orion, and if so, what changes are needed? I know Marvell has contracted you to port to Armada 370/XP, not mvebu, aka all chips which might be able to share this code. This IMHO is wrong. So i personally would put the kirkwood and the Armada 370/XP data sheets next to each other, and from the beginning on write the driver so it supports them all. This assumes the ASIC engineers have not radically changed anything... Please also try to stick to the DT binding i've used. If however, you have to focus solely on Armada 370/XP, then the community can work on making your code generic at a late date. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers
Hello Andrew, Le Tue, 3 Jul 2012 16:22:34 +0200, Andrew Lunn a écrit : > Both IRQ and GPIO controllers can now be represented in DT. The IRQ > controllers are setup first, and then the GPIO controllers. Interrupts > for GPIO lines are placed directly after the main interrupts in the > interrupt space. > > Signed-off-by: Andrew Lunn I have started working on a pinctrl driver for mvebu, which would handle pin muxing (MPP) + gpio + gpio interrupts. So far, the pin muxing part is working (needs some polishing, but the foundation is here), with device tree bindings. I think the pin muxing could be used for Orion as well. Now, I'm planning to start working on the gpio + gpio interrupts parts of the driver, and I'm wondering how to interact with your work on the matter. My understanding is that the new way of doing a pinmux+gpio driver is to implement it in drivers/pinctrl/, which I have started doing. Should I continue working on a drivers/pinctrl/ driver for mvebu for Armada 370/XP, and then we see together if it makes sense to extend to Orion, and if so, what changes are needed? Thanks, Thomas -- Thomas Petazzoni, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
