Re: [PATCH v4 4/5] ARM: mediatek: Add EINT support to MTK pinctrl driver.

Re: [PATCH v4 4/5] ARM: mediatek: Add EINT support to MTK pinctrl driver.

[Yingjoe Chen]

On Wed, 2014-12-17 at 07:34 +0800, Hongzhou Yang wrote:
> From: Maoguang Meng <maoguang.meng@mediatek.com>
> 
> MTK SoC support external interrupt(EINT) from most SoC pins.
> Add EINT support to pinctrl driver.
> 
> Signed-off-by: Maoguang Meng <maoguang.meng@mediatek.com>
> Signed-off-by: Hongzhou Yang <hongzhou.yang@mediatek.com>

Hi Linus,

This patch add EINT support to the pinctrl driver. We've surveyed
GPIOLIB_IRQCHIP, but we didn't use it because:

- Not every GPIO pin support interrupt.
- EINT use a different numbering to GPIO. eg, from the mt8135 table,
GPIO29 is EINT158. It is more nature & efficient to use EINT number as
hwirq.

+               MTK_EINT_FUNCTION(2, 158),
+               MTK_FUNCTION(0, "GPIO29"),

Joe.C

Re: [PATCH v4 4/5] ARM: mediatek: Add EINT support to MTK pinctrl driver.

[Yingjoe Chen]

On Wed, 2014-12-17 at 17:09 +0800, Yingjoe Chen wrote:
> On Wed, 2014-12-17 at 07:34 +0800, Hongzhou Yang wrote:
> > From: Maoguang Meng <maoguang.meng@mediatek.com>
> > 
> > MTK SoC support external interrupt(EINT) from most SoC pins.
> > Add EINT support to pinctrl driver.
> > 
> > Signed-off-by: Maoguang Meng <maoguang.meng@mediatek.com>
> > Signed-off-by: Hongzhou Yang <hongzhou.yang@mediatek.com>
> 
> Hi Linus,
> 
> This patch add EINT support to the pinctrl driver. We've surveyed
> GPIOLIB_IRQCHIP, but we didn't use it because:
> 
> - Not every GPIO pin support interrupt.
> - EINT use a different numbering to GPIO. eg, from the mt8135 table,
> GPIO29 is EINT158. It is more nature & efficient to use EINT number as
> hwirq.
> 
> +               MTK_EINT_FUNCTION(2, 158),
> +               MTK_FUNCTION(0, "GPIO29"),


Hi Linus,

After further looking into this, we could use GPIOLIB_IRQCHIP if we add
an extension gpiochip_irqchip_add() to accept interrupt numbers and
custom .to_irq function for our SoC. We could still reuse other code
GPIOLIB_IRQCHIP provide.

Please let me know what you think about this idea.
Thanks.

Joe.C

Re: [PATCH v4 4/5] ARM: mediatek: Add EINT support to MTK pinctrl driver.

[Linus Walleij]

On Tue, Jan 6, 2015 at 10:16 AM, Yingjoe Chen <yingjoe.chen@mediatek.com> wrote:
> On Wed, 2014-12-17 at 17:09 +0800, Yingjoe Chen wrote:
>> On Wed, 2014-12-17 at 07:34 +0800, Hongzhou Yang wrote:
>> > From: Maoguang Meng <maoguang.meng@mediatek.com>
>> >
>> > MTK SoC support external interrupt(EINT) from most SoC pins.
>> > Add EINT support to pinctrl driver.
>> >
>> > Signed-off-by: Maoguang Meng <maoguang.meng@mediatek.com>
>> > Signed-off-by: Hongzhou Yang <hongzhou.yang@mediatek.com>
>>
>> Hi Linus,
>>
>> This patch add EINT support to the pinctrl driver. We've surveyed
>> GPIOLIB_IRQCHIP, but we didn't use it because:
>>
>> - Not every GPIO pin support interrupt.
>> - EINT use a different numbering to GPIO. eg, from the mt8135 table,
>> GPIO29 is EINT158. It is more nature & efficient to use EINT number as
>> hwirq.
>>
>> +               MTK_EINT_FUNCTION(2, 158),
>> +               MTK_FUNCTION(0, "GPIO29"),
>
> After further looking into this, we could use GPIOLIB_IRQCHIP if we add
> an extension gpiochip_irqchip_add() to accept interrupt numbers and
> custom .to_irq function for our SoC. We could still reuse other code
> GPIOLIB_IRQCHIP provide.

I see, and I still want to see all possibilities to centralize code
surveyed.

If I understand correctly what you actually need is a linear
irqdomain with "holes" (invalid offsets) in it.
So this is what we should design for.

The .to_irq() function should not really perform anything but a
simple lookup in the domain.

What you do here:

+static int mtk_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
+{
+       const struct mtk_desc_pin *pin;
+       struct mtk_pinctrl *pctl = dev_get_drvdata(chip->dev);
+       int irq;
+
+       pin = pctl->devdata->pins + offset;
+       if (pin->eint.eintnum == NO_EINT_SUPPORT)
+               return -EINVAL;
+
+       irq = irq_find_mapping(pctl->domain, pin->eint.eintnum);
+       if (!irq)
+               return -EINVAL;
+
+       return irq;
+}

Is *avoiding* to translate some IRQs from a certain offset using
the domain, I think that is the wrong way to go.

Instead, we would need to handle these "holes" in
drivers/pinctrl/nomadik/pinctrl-abx500.c also need the same
thing.

Maybe we can add a gpiochip_irqchip_add_sparse() in
gpiolib that will take an array of bools in, indicating of
a certain line is elegible for IRQ or not, e.g.:

bool valid = { false, true, false, true };

foo = gpiochip_irqchip_add_sparse(&chip,
                         &irqchip,
                         0,
                         handler,
                         IRQ_TYPE_NONE,
                         &valid);

So the loop inside the lib will avoid calling
irq_create_mapping() on the invalid lines, and any calls
to irq_find_mapping() will fail if you try to use one of
them for IRQs.

If you're up to do this and test on your driver I'm all in for it.

Yours,
Linus Walleij

Re: [PATCH v4 4/5] ARM: mediatek: Add EINT support to MTK pinctrl driver.

[Yingjoe Chen]

On Tue, 2015-01-13 at 14:24 +0100, Linus Walleij wrote:
> On Tue, Jan 6, 2015 at 10:16 AM, Yingjoe Chen <yingjoe.chen@mediatek.com> wrote:
> > On Wed, 2014-12-17 at 17:09 +0800, Yingjoe Chen wrote:
> >> On Wed, 2014-12-17 at 07:34 +0800, Hongzhou Yang wrote:
> >> > From: Maoguang Meng <maoguang.meng@mediatek.com>
> >> >
> >> > MTK SoC support external interrupt(EINT) from most SoC pins.
> >> > Add EINT support to pinctrl driver.
> >> >
> >> > Signed-off-by: Maoguang Meng <maoguang.meng@mediatek.com>
> >> > Signed-off-by: Hongzhou Yang <hongzhou.yang@mediatek.com>
> >>
> >> Hi Linus,
> >>
> >> This patch add EINT support to the pinctrl driver. We've surveyed
> >> GPIOLIB_IRQCHIP, but we didn't use it because:
> >>
> >> - Not every GPIO pin support interrupt.
> >> - EINT use a different numbering to GPIO. eg, from the mt8135 table,
> >> GPIO29 is EINT158. It is more nature & efficient to use EINT number as
> >> hwirq.
> >>
> >> +               MTK_EINT_FUNCTION(2, 158),
> >> +               MTK_FUNCTION(0, "GPIO29"),
> >
> > After further looking into this, we could use GPIOLIB_IRQCHIP if we add
> > an extension gpiochip_irqchip_add() to accept interrupt numbers and
> > custom .to_irq function for our SoC. We could still reuse other code
> > GPIOLIB_IRQCHIP provide.
> 
> I see, and I still want to see all possibilities to centralize code
> surveyed.
> 
> If I understand correctly what you actually need is a linear
> irqdomain with "holes" (invalid offsets) in it.
> So this is what we should design for.
> 
> The .to_irq() function should not really perform anything but a
> simple lookup in the domain.
> 
> What you do here:
> 
> +static int mtk_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
> +{
> +       const struct mtk_desc_pin *pin;
> +       struct mtk_pinctrl *pctl = dev_get_drvdata(chip->dev);
> +       int irq;
> +
> +       pin = pctl->devdata->pins + offset;
> +       if (pin->eint.eintnum == NO_EINT_SUPPORT)
> +               return -EINVAL;
> +
> +       irq = irq_find_mapping(pctl->domain, pin->eint.eintnum);
> +       if (!irq)
> +               return -EINVAL;
> +
> +       return irq;
> +}
> 
> Is *avoiding* to translate some IRQs from a certain offset using
> the domain, I think that is the wrong way to go.

Hi Linus,

Yes, it have holes and avoiding translate some gpio to irq, but it also
using a different hwirq number to do the translate.

Let's me describe my problem more clearly. On our SoC, if a pin support
interrupt it will have 2 different numbers for it. For examples, here's
a partial list for the gpio and EINT number mappings on mt8135:

   gpio  EINT
     0     49
     1     48
    ...........
    36     97
    37     19      
    ...........

To control interrupt related function, we'll need EINT number to locate
corresponding register bits. When interrupt occurs, the interrupt
handler will know which EINT interrupt occurs. In irq_chip functions,
only .irq_request_resources and .irq_release_resources use gpio number
to set pinmux to EINT mode and all the others need EINT number.

Because EINT number is used more frequently in interrupt related
functions, it make sense to use EINT number as hwirq instead of gpio
number. That means irq_domain will translate EINT number to virq.
So what mtk_gpio_to_irq actually do is translate gpio number to EINT
number and use irq domain to translate it to virq.

Below is a draft of what I have in mind. For SoC that can use gpio
number to control irq they still use gpiochip_irqchip_add(). For SoC
that need to use another number to control irq, like us, can use
gpiochip_irqchip_add_with_map. We can't reuse gpiochip_to_irq or
gpiochip_irq_reqres/relres in GPIOLIB_IRQCHIP, but we can still reuse
others code.

Let me know if this is the direction you want.
Thanks

Joe.C

=======================
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -567,11 +567,14 @@ static void gpiochip_irqchip_remove(struct
gpio_chip *gpiochip)
  * the pins on the gpiochip can generate a unique IRQ. Everything else
  * need to be open coded.
  */
-int gpiochip_irqchip_add(struct gpio_chip *gpiochip,
-                        struct irq_chip *irqchip,
-                        unsigned int first_irq,
-                        irq_flow_handler_t handler,
-                        unsigned int type)
+int gpiochip_irqchip_add_with_map(struct gpio_chip *gpiochip,
+                                 struct irq_chip *irqchip,
+                                 unsigned int first_irq,
+                                 irq_flow_handler_t handler,
+                                 unsigned int type,
+                                 unsigned int size,
+                                 int (*to_irq_func)(struct gpio_chip *chip,
+                                                    unsigned offset))
 {
        struct device_node *of_node;
        unsigned int offset;
@@ -604,15 +607,17 @@ int gpiochip_irqchip_add(struct gpio_chip *gpiochip,
                gpiochip->irqchip = NULL;
                return -EINVAL;
        }
-       irqchip->irq_request_resources = gpiochip_irq_reqres;
-       irqchip->irq_release_resources = gpiochip_irq_relres;
+       if (!irqchip->irq_request_resources) {
+               irqchip->irq_request_resources = gpiochip_irq_reqres;
+               irqchip->irq_release_resources = gpiochip_irq_relres;
+       }

        /*
         * Prepare the mapping since the irqchip shall be orthogonal to
         * any gpiochip calls. If the first_irq was zero, this is
         * necessary to allocate descriptors for all IRQs.
         */
-       for (offset = 0; offset < gpiochip->ngpio; offset++) {
+       for (offset = 0; offset < size; offset++) {
                irq_base = irq_create_mapping(gpiochip->irqdomain, offset);
                if (offset == 0)
                        /*
@@ -626,6 +631,18 @@ int gpiochip_irqchip_add(struct gpio_chip *gpiochip,

        return 0;
 }
+
+int gpiochip_irqchip_add(struct gpio_chip *gpiochip,
+                        struct irq_chip *irqchip,
+                        unsigned int first_irq,
+                        irq_flow_handler_t handler,
+                        unsigned int type)
+{
+       return gpiochip_irqchip_add_with_map(gpiochip, irqchip, first_irq,
+                                            handler, type,
+                                            gpiochip->ngpiog, piochip_to_irq);
+}

Re: [PATCH v4 4/5] ARM: mediatek: Add EINT support to MTK pinctrl driver.

[Linus Walleij]

On Wed, Jan 14, 2015 at 3:32 AM, Yingjoe Chen <yingjoe.chen@mediatek.com> wrote:

> Let's me describe my problem more clearly. On our SoC, if a pin support
> interrupt it will have 2 different numbers for it. For examples, here's
> a partial list for the gpio and EINT number mappings on mt8135:
>
>    gpio  EINT
>      0     49
>      1     48
>     ...........
>     36     97
>     37     19
>     ...........
>
> To control interrupt related function, we'll need EINT number to locate
> corresponding register bits. When interrupt occurs, the interrupt
> handler will know which EINT interrupt occurs. In irq_chip functions,
> only .irq_request_resources and .irq_release_resources use gpio number
> to set pinmux to EINT mode and all the others need EINT number.
>
> Because EINT number is used more frequently in interrupt related
> functions, it make sense to use EINT number as hwirq instead of gpio
> number. That means irq_domain will translate EINT number to virq.
> So what mtk_gpio_to_irq actually do is translate gpio number to EINT
> number and use irq domain to translate it to virq.

But the EINT is not a hardware number is it?

That sounds like an abuse of the irqdomain framework.

The purpose of that code is to map hardware IRQs to Linux
IRQs nothing else.

This sounds more like mapping a Linux IRQ (the EINT) to
another Linux IRQ (whatever the irqdomain allocates for
you).

Since the EINTs are already Linux IRQs, these should be used
directly.

I would solve this by just having some cross-reference table
for mapping GPIOs to EINTs without involving the irqdomain
at all.

struct eint_map {
    int gpio_offset;
    int eint;
};

But also check with the irqdomain people.

Yours,
Linus Walleij

Re: [PATCH v4 4/5] ARM: mediatek: Add EINT support to MTK pinctrl driver.

[Yingjoe Chen]

On Thu, 2015-01-15 at 18:30 +0100, Linus Walleij wrote:
> On Wed, Jan 14, 2015 at 3:32 AM, Yingjoe Chen <yingjoe.chen@mediatek.com> wrote:
> 
> > Let's me describe my problem more clearly. On our SoC, if a pin support
> > interrupt it will have 2 different numbers for it. For examples, here's
> > a partial list for the gpio and EINT number mappings on mt8135:
> >
> >    gpio  EINT
> >      0     49
> >      1     48
> >     ...........
> >     36     97
> >     37     19
> >     ...........
> >
> > To control interrupt related function, we'll need EINT number to locate
> > corresponding register bits. When interrupt occurs, the interrupt
> > handler will know which EINT interrupt occurs. In irq_chip functions,
> > only .irq_request_resources and .irq_release_resources use gpio number
> > to set pinmux to EINT mode and all the others need EINT number.
> >
> > Because EINT number is used more frequently in interrupt related
> > functions, it make sense to use EINT number as hwirq instead of gpio
> > number. That means irq_domain will translate EINT number to virq.
> > So what mtk_gpio_to_irq actually do is translate gpio number to EINT
> > number and use irq domain to translate it to virq.
> 
> But the EINT is not a hardware number is it?

It is a hardware number. eg, to mask irq for the gpio 0 above, we have
to set bit49 in EINT mask_set register. When this irq triggered, it is
reported using EINT status register bit49.

We can find out EINT number for a GPIO using mtk_desc_pin tables. In
drivers/pinctrl/mediatek/pinctrl-mtk-mt8135.h, for GPIO0, we can see its
EINT is 49 and must set to function 2 for EINT. I believe this is
similar to sunxi.

        MTK_PIN(
                PINCTRL_PIN(0, "MSDC0_DAT7"),
                "D21", "mt8135",
                MTK_EINT_FUNCTION(2, 49),
                MTK_FUNCTION(0, "GPIO0"),
                MTK_FUNCTION(1, "MSDC0_DAT7"),
                MTK_FUNCTION(2, "EINT49"),
                MTK_FUNCTION(3, "I2SOUT_DAT"),
                MTK_FUNCTION(4, "DAC_DAT_OUT"),
                MTK_FUNCTION(5, "PCM1_DO"),
                MTK_FUNCTION(6, "SPI1_MO"),
                MTK_FUNCTION(7, "NALE")
        ),

Joe.C

Re: [PATCH v4 4/5] ARM: mediatek: Add EINT support to MTK pinctrl driver.

[Linus Walleij]

On Fri, Jan 16, 2015 at 3:50 AM, Yingjoe Chen <yingjoe.chen@mediatek.com> wrote:
> On Thu, 2015-01-15 at 18:30 +0100, Linus Walleij wrote:

>> But the EINT is not a hardware number is it?
>
> It is a hardware number. eg, to mask irq for the gpio 0 above, we have
> to set bit49 in EINT mask_set register. When this irq triggered, it is
> reported using EINT status register bit49.

OK I get it (finally) then you should indeed use the irqdomain for it...
It's just a bit confusing that it begins at offset 49...

Yours,
Linus Walleij

Re: [PATCH v4 1/5] ARM: mediatek: Add config options for mediatek SoCs.

[Linus Walleij]

On Wed, Dec 17, 2014 at 12:34 AM, Hongzhou Yang
<hongzhou.yang@mediatek.com> wrote:

> From: Yingjoe Chen <yingjoe.chen@mediatek.com>
>
> The upcoming MTK pinctrl driver have a big pin table for each SoC
> and we don't want to bloat the kernel binary if we don't need it.
> Add config options so we can build for one SoC only.
>
> Signed-off-by: Yingjoe Chen <yingjoe.chen@mediatek.com>
> Signed-off-by: Hongzhou Yang <hongzhou.yang@mediatek.com>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

I guess this goes directly to the ARM SoC tree. As it is only
Kconfig symbols it should be OK to merge out-of-order.

Yours,
Linus Walleij

Re: [PATCH v4 1/5] ARM: mediatek: Add config options for mediatek SoCs.

[Yingjoe Chen]

On Tue, 2015-01-13 at 10:43 +0100, Linus Walleij wrote:
> On Wed, Dec 17, 2014 at 12:34 AM, Hongzhou Yang
> <hongzhou.yang@mediatek.com> wrote:
> 
> > From: Yingjoe Chen <yingjoe.chen@mediatek.com>
> >
> > The upcoming MTK pinctrl driver have a big pin table for each SoC
> > and we don't want to bloat the kernel binary if we don't need it.
> > Add config options so we can build for one SoC only.
> >
> > Signed-off-by: Yingjoe Chen <yingjoe.chen@mediatek.com>
> > Signed-off-by: Hongzhou Yang <hongzhou.yang@mediatek.com>
> 
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> 
> I guess this goes directly to the ARM SoC tree. As it is only
> Kconfig symbols it should be OK to merge out-of-order.
> 

Hi Matthias,

Since this one is in arch/arm/mach-mediatek, can you take it?

Links for original post in case you can't find it.
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-December/311181.html

Joe.C

Re: [PATCH v4 2/5] dt-bindings: Add pinctrl bindings for mt65xx/mt81xx.

[Linus Walleij]

On Wed, Dec 17, 2014 at 12:34 AM, Hongzhou Yang
<hongzhou.yang@mediatek.com> wrote:

> From: Hongzhou Yang <hongzhou.yang@mediatek.com>
>
> Add devicetree bindings for Mediatek SoC pinctrl driver.
>
> Signed-off-by: Hongzhou Yang <hongzhou.yang@mediatek.com>

So following from the discussion with Sascha...

(...)
> diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-mt65xx.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt65xx.txt
> new file mode 100644
> index 0000000..3bf34f9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt65xx.txt
> @@ -0,0 +1,142 @@
> +* Mediatek MT65XX Pin Controller
> +
> +The Mediatek's Pin controller is used to control SoC pins.
> +
> +Required properties:
> +- compatible: value should be either of the following.
> +    (a) "mediatek,mt8135-pinctrl", compatible with mt8135 pinctrl.
> +- mediatek,pctl-regmap: Should be a phandle of the syscfg node.
> +- gpio-controller : Marks the device node as a gpio controller.
> +- #gpio-cells: number of cells in GPIO specifier. Since the generic GPIO
> +  binding is used, the amount of cells must be specified as 2. See the below
> +  mentioned gpio binding representation for description of particular cells.
> +
> +       Eg: <&pio 6 0>
> +       <[phandle of the gpio controller node]
> +       [pin number within the gpio controller]

Call this "line number", not "pin number" as GPIO != pin

> +       [flags]>
> +
> +       Values for gpio specifier:
> +       - Pin number: is a value between 0 to 202.

Line number.

> +       - Flags:  bit field of flags, as defined in <dt-bindings/gpio/gpio.h>.
> +            Only the following flags are supported:
> +            0 - GPIO_ACTIVE_HIGH
> +            1 - GPIO_ACTIVE_LOW
> +- reg: physicall address base for EINT registers
> +- interrupt-controller: Marks the device node as an interrupt controller
> +- #interrupt-cells: Should be two.
> +- interrupts : The interrupt outputs from the controller.
> +
> +Please refer to pinctrl-bindings.txt in this directory for details of the
> +common pinctrl bindings used by client devices.
> +
> +A pinctrl node should contain at least one subnodes representing the
> +pinctrl groups available on the machine. Each subnode will list the
> +pins it needs, and how they should be configured, with regard to muxer
> +configuration, pullups, drive strngth, input enable/disable and input schmitt.
> +
> +Required subnode-properties:
> +
> +- pins: 2 integers array, represents gpio pinmux number and config
> +  setting. The format as following
> +
> +    node {
> +     pins = <PIN_NUMBER_PINMUX>;
> +                     GENERIC_PINCONFIG;
> +    };

As discussed with sascha, pins = ... is for pins specified with a string.

Either

- Specify your pins with a string

- Come up wiyj some new unique way to name pins identfied by
  numbers, maybe just "pinno = <...>" Sascha suggested "pinmux"
  but that is strange if the number is used for pin config.

(Check with Sascha!)

> +    The PIN_NUMBER_PINMUX is combination of GPIO number and pinmux, it can
> +    use macros which already defind in boot/dts/mt8135-pinfunc.h directly.

So is that prop to be named pinmux, pinno, pinid or something..

> +    The GENERIC_PINCONFIG is the generic pinconfig options to use, bias-disable,
> +    bias-pull-down, bias-pull-up, input-enable, input-disable, output-low, output-high,
> +    input-schmitt-enable, input-schmitt-disable and drive-strength are valid.

Nice!

> +       pinctrl@01c20800 {
> +               compatible = "mediatek,mt8135-pinctrl";
> +               reg = <0 0x1000B000 0 0x1000>;
> +               mediatek,pctl-regmap = <&syscfg_pctl_a &syscfg_pctl_b>;
> +               gpio-controller;
> +               #gpio-cells = <2>;
> +               interrupt-controller;
> +               #interrupt-cells = <2>;
> +               interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>,
> +                               <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>,
> +                               <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>;
> +
> +               i2c0_pins_a: i2c0@0 {
> +                       pins1 {
> +                               pins = <MT8135_PIN_100_SDA0__FUNC_SDA0>,
> +                                               <MT8135_PIN_101_SCL0__FUNC_SCL0>;

So this prop need to be renamed.

> +                               bias-disable;
> +                       };
> +               };

Yours,
Linus Walleij

Re: [PATCH v4 2/5] dt-bindings: Add pinctrl bindings for mt65xx/mt81xx.

[Matthias Brugger]

2014-12-17 0:34 GMT+01:00 Hongzhou Yang <hongzhou.yang@mediatek.com>:
> From: Hongzhou Yang <hongzhou.yang@mediatek.com>
>
> Add devicetree bindings for Mediatek SoC pinctrl driver.
>
> Signed-off-by: Hongzhou Yang <hongzhou.yang@mediatek.com>
> ---
>  .../devicetree/bindings/pinctrl/pinctrl-mt65xx.txt | 142 +++++++++++++++++++++
>  1 file changed, 142 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/pinctrl-mt65xx.txt
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-mt65xx.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt65xx.txt
> new file mode 100644
> index 0000000..3bf34f9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt65xx.txt
> @@ -0,0 +1,142 @@
> +* Mediatek MT65XX Pin Controller
> +
> +The Mediatek's Pin controller is used to control SoC pins.
> +
> +Required properties:
> +- compatible: value should be either of the following.
> +    (a) "mediatek,mt8135-pinctrl", compatible with mt8135 pinctrl.
> +- mediatek,pctl-regmap: Should be a phandle of the syscfg node.
> +- gpio-controller : Marks the device node as a gpio controller.
> +- #gpio-cells: number of cells in GPIO specifier. Since the generic GPIO
> +  binding is used, the amount of cells must be specified as 2. See the below
> +  mentioned gpio binding representation for description of particular cells.
> +
> +       Eg: <&pio 6 0>
> +       <[phandle of the gpio controller node]
> +       [pin number within the gpio controller]
> +       [flags]>
> +
> +       Values for gpio specifier:
> +       - Pin number: is a value between 0 to 202.
> +       - Flags:  bit field of flags, as defined in <dt-bindings/gpio/gpio.h>.
> +            Only the following flags are supported:
> +            0 - GPIO_ACTIVE_HIGH
> +            1 - GPIO_ACTIVE_LOW
> +- reg: physicall address base for EINT registers
> +- interrupt-controller: Marks the device node as an interrupt controller
> +- #interrupt-cells: Should be two.
> +- interrupts : The interrupt outputs from the controller.
> +
> +Please refer to pinctrl-bindings.txt in this directory for details of the
> +common pinctrl bindings used by client devices.
> +
> +A pinctrl node should contain at least one subnodes representing the
> +pinctrl groups available on the machine. Each subnode will list the
> +pins it needs, and how they should be configured, with regard to muxer
> +configuration, pullups, drive strngth, input enable/disable and input schmitt.

drive strength

> +
> +Required subnode-properties:
> +
> +- pins: 2 integers array, represents gpio pinmux number and config
> +  setting. The format as following
> +
> +    node {
> +     pins = <PIN_NUMBER_PINMUX>;
> +                     GENERIC_PINCONFIG;
> +    };
> +
> +    The PIN_NUMBER_PINMUX is combination of GPIO number and pinmux, it can
> +    use macros which already defind in boot/dts/mt8135-pinfunc.h directly.
> +    The GENERIC_PINCONFIG is the generic pinconfig options to use, bias-disable,
> +    bias-pull-down, bias-pull-up, input-enable, input-disable, output-low, output-high,
> +    input-schmitt-enable, input-schmitt-disable and drive-strength are valid.
> +
> +    Some special pins have extra pull up strength, there are R0 and R1 pull-up
> +    resistors available, but for user, it's only need to set R1R0 as 00, 01, 10 or 11.
> +    So when config bias-pull-up, it support arguments for those special pins.
> +    Some macros have been defined for this usage, such as MTK_PUPD_SET_R1R0_00.
> +    See dt-bindings/pinctrl/mt65xx.h.
> +
> +    When config drive-strength, it can support some arguments, such as
> +    MTK_DRIVE_4mA, MTK_DRIVE_6mA, etc. See dt-bindings/pinctrl/mt65xx.h.
> +
> +Examples:
> +
> +#include "mt8135-pinfunc.h"
> +
> +...
> +{
> +       syscfg_pctl_a: syscfg_pctl_a@10005000 {
> +               compatible = "mediatek,mt8135-pctl-a-syscfg", "syscon";
> +               reg = <0 0x10005000 0 0x1000>;
> +       };
> +
> +       syscfg_pctl_b: syscfg_pctl_b@1020C020 {
> +               compatible = "mediatek,mt8135-pctl-b-syscfg", "syscon";
> +               reg = <0 0x1020C020 0 0x1000>;
> +       };
> +
> +       pinctrl@01c20800 {
> +               compatible = "mediatek,mt8135-pinctrl";
> +               reg = <0 0x1000B000 0 0x1000>;
> +               mediatek,pctl-regmap = <&syscfg_pctl_a &syscfg_pctl_b>;
> +               gpio-controller;
> +               #gpio-cells = <2>;
> +               interrupt-controller;
> +               #interrupt-cells = <2>;
> +               interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>,
> +                               <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>,
> +                               <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>;
> +
> +               i2c0_pins_a: i2c0@0 {
> +                       pins1 {
> +                               pins = <MT8135_PIN_100_SDA0__FUNC_SDA0>,
> +                                               <MT8135_PIN_101_SCL0__FUNC_SCL0>;
> +                               bias-disable;
> +                       };
> +               };
> +
> +               i2c1_pins_a: i2c1@0 {
> +                       pins {
> +                               pins = <MT8135_PIN_195_SDA1__FUNC_SDA1>,
> +                                               <MT8135_PIN_196_SCL1__FUNC_SCL1>;
> +                               bias-pull-up = <55>;
> +                       };
> +               };
> +
> +               i2c2_pins_a: i2c2@0 {
> +                       pins1 {
> +                               pins = <MT8135_PIN_193_SDA2__FUNC_SDA2>;
> +                               bias-pull-down;
> +                       };
> +
> +                       pins2 {
> +                               pins = <MT8135_PIN_49_WATCHDOG__FUNC_GPIO49>;
> +                               bias-pull-up;
> +                       };
> +               };
> +
> +               i2c3_pins_a: i2c3@0 {
> +                       pins1 {
> +                               pins = <MT8135_PIN_40_DAC_CLK__FUNC_GPIO40>,
> +                                               <MT8135_PIN_41_DAC_WS__FUNC_GPIO41>;
> +                               bias-pull-up = <55>;
> +                       };
> +
> +                       pins2 {
> +                               pins = <MT8135_PIN_35_SCL3__FUNC_SCL3>,
> +                                               <MT8135_PIN_36_SDA3__FUNC_SDA3>;
> +                               output-low;
> +                               bias-pull-up = <55>;
> +                       };
> +
> +                       pins3 {
> +                               pins = <MT8135_PIN_57_JTCK__FUNC_GPIO57>,
> +                                               <MT8135_PIN_60_JTDI__FUNC_JTDI>;
> +                               drive-strength = <32>;
> +                       };
> +               };
> +
> +               ...
> +       }
> +};
> --
> 1.8.1.1.dirty
>



-- 
motzblog.wordpress.com

Re: [PATCH v4 5/5] ARM: dts: mt8135: Add pinctrl/GPIO/EINT node for mt8135.

[Matthias Brugger]

2014-12-17 0:34 GMT+01:00 Hongzhou Yang <hongzhou.yang@mediatek.com>:
> From: Hongzhou Yang <hongzhou.yang@mediatek.com>
>
> Add pinctrl,GPIO and EINT node to mt8135.dtsi.
>
> Signed-off-by: Hongzhou Yang <hongzhou.yang@mediatek.com>
> ---
>  arch/arm/boot/dts/mt8135-pinfunc.h | 1304 ++++++++++++++++++++++++++++++++++++
>  arch/arm/boot/dts/mt8135.dtsi      |   24 +
>  2 files changed, 1328 insertions(+)
>  create mode 100644 arch/arm/boot/dts/mt8135-pinfunc.h
>
[...]
> diff --git a/arch/arm/boot/dts/mt8135.dtsi b/arch/arm/boot/dts/mt8135.dtsi
> index 90a56ad..38e3469 100644
> --- a/arch/arm/boot/dts/mt8135.dtsi
> +++ b/arch/arm/boot/dts/mt8135.dtsi
> @@ -15,6 +15,7 @@
>  #include <dt-bindings/interrupt-controller/irq.h>
>  #include <dt-bindings/interrupt-controller/arm-gic.h>
>  #include "skeleton64.dtsi"
> +#include "mt8135-pinfunc.h"
>
>  / {
>         compatible = "mediatek,mt8135";
> @@ -94,6 +95,16 @@
>                 compatible = "simple-bus";
>                 ranges;
>
> +               syscfg_pctl_a: syscfg_pctl_a@10005000 {
> +                       compatible = "mediatek,mt8135-pctl-a-syscfg", "syscon";
> +                       reg = <0 0x10005000 0 0x1000>;
> +               };
> +
> +               syscfg_pctl_b: syscfg_pctl_b@1020C000 {
> +                       compatible = "mediatek,mt8135-pctl-b-syscfg", "syscon";
> +                       reg = <0 0x1020C000 0 0x1000>;
> +               };
> +
>                 timer: timer@10008000 {
>                         compatible = "mediatek,mt8135-timer", "mediatek,mt6577-timer";
>                         reg = <0 0x10008000 0 0x80>;
> @@ -111,5 +122,18 @@
>                               <0 0x10214000 0 0x2000>,
>                               <0 0x10216000 0 0x2000>;
>                 };
> +
> +               pio: pinctrl@10005000 {
> +                       compatible = "mediatek,mt8135-pinctrl";
> +                       reg = <0 0x1000B000 0 0x1000>;
> +                       mediatek,pctl-regmap = <&syscfg_pctl_a &syscfg_pctl_b>;
> +                       gpio-controller;
> +                       #gpio-cells = <2>;
> +                       interrupt-controller;
> +                       #interrupt-cells = <2>;
> +                       interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>;
> +               };

Please order the nodes taking into account the memory address to which
they are mapped.

Cheers,
Matthias


-- 
motzblog.wordpress.com