Re: [PATCH 1/2] pinmux: Add TB10x pinmux driver

[Christian Ruppert]

On Wed, May 08, 2013 at 02:01:53PM -0600, Stephen Warren wrote:
> On 05/08/2013 10:41 AM, Christian Ruppert wrote:
> ...
> > What do you think about the following modification to the pinctrl/GPIO
> > frameworks instead (not yet a formal patch, more a request for comment
> > to illustrate what I mean. If you agree, I will clean it up and submit a
> > proper patch after discussion).
> > 
> > It adds a dt_gpiorange_xlate function to the pinctrl callbacks which
> > defaults to the conventional behaviour using kernel logical pin numbers.
> > However, pin controllers which provide more complex mechanisms can
> > define #gpio-range-cells and provide this callback in order to keep
> > Linux pin numbering inside the kernel.
> 
> Can you provide an example of the DT content, and explain exactly what
> this patch does with it; what effect it has on the existing GPIO or
> pinctrl code?

The patch does not change the default behaviour of the kernel: In case
no dt_gpiorange_xlate callback is defined for a given driver (e.g. for
pre-existing drivers), the default function simply interprets the first
argument as Linux pin number and the second as pin count, same as now.
New drivers can use the callback to translate device specific pin
references to Linux pin numbers (in the idea of of_xlate in the GPIO
framework or xlate in the irqchip framework).

In the case of TB10x, I was thinking of something in the lines of

iomux: iomux@FF10601c {
	#gpio-range-cells = <1>;   /* one cell used for gpiorange phandle */
	compatible = "abilis,tb10x-iomux";
	reg = <0xFF10601c 0x4>;
	pctl_gpio_a: pctl-gpio-a { /* define phandle to GPIOA I/O function */
		pingrp = "gpioa_pins";
	};
	pctl_gpio_b: pctl-gpio-b { /* define phandle to GPIOB I/O function */
		pingrp = "gpiob_pins";
	};
	pctl_uart0: pctl-uart0 {   /* define phandle to UART0 I/O function */
		pingrp = "uart0_pins";
	};
};
uart@FF100000 {
	compatible = "snps,dw-apb-uart";
	reg = <0xFF100000 0x100>;
	clock-frequency = <166666666>;
	interrupts = <25 1>;
	reg-shift = <2>;
	reg-io-width = <4>;
	pinctrl-names = "default";  /* For non-GPIO modules, request I/O */
	pinctrl-0 = <&pctl_uart0>;  /* functions normally */
};
gpioa: gpio@FF140000 {
	compatible = "abilis,tb10x-gpio";
	reg = <0xFF140000 0x1000>;
	gpio-controller;
	#gpio-cells = <2>;
	gpio-ranges = <&iomux &pctl_gpio_a>; /* (*1) */
};

gpioa: gpio@FF140000 {
	compatible = "abilis,tb10x-gpio";
	reg = <0xFF140000 0x1000>;
	gpio-controller;
	#gpio-cells = <2>;
	pinctrl-names = "default"   /* here the GPIO controller requests */
	pinctrl-0 = <&pctl_gpio_b>; /* the entire GPIOB port explicitly */
	gpio-ranges = <&iomux &pctl_gpio_b>; /* (*2) */
};

(*1) TB100 GPIO ranges are defined as a phandle to the I/O function
     which provides all pins of a given GPIO port. This function is not
     necessarily requested from pinctrl and GPIO ports may overlap with
     other functions. The pin controller knows about this and provides
     whatever GPIO pin is available after mapping other requested
     functions.
(*2) Here, the entire GPIOB port is explicitly requested by the GPIO
     module, i.e. all pins of the port are made available as GPIOs.

The xlate function in pinctrl-tb10x.c would look something like this:

static int tb10x_dt_gpiorange_xlate(struct pinctrl_dev *pctldev,
					struct device_node *np,
					u32 *rangespec, int rangespecsize,
					u32 *pin_offset, u32 *npins)
{
	const char *name;
	int ret;
	struct tb10x_pinfuncgrp *pfg;
	struct device_node *dn;

	if (rangespecsize != 1)
		return -EINVAL;

	dn = of_find_node_by_phandle(rangespec[0]);
	if (!dn)
		return -EINVAL;
	of_node_put(dn);

	ret = of_property_read_string(dn, "pingrp", &name);
	if (ret)
		return ret;

	pfg = tb10x_get_pinfuncgrp(name);
	if (IS_ERR(pfg))
		return PTR_ERR(pfg);

	if (!pfg->isgpio)
		return -EINVAL;

	*pin_offset = pfg->pins[0];
	*npins      = pfg->pincnt;

	return 0;
}

Other drivers may use physical pin numbers, assign integer identifiers
to each GPIO bank or use some other numbering scheme documented in
Documentation/devicetree/bindings.

Greetings,
  Christian

-- 
  Christian Ruppert              ,          <christian.ruppert@xxxxxxxxxx>
                                /|
  Tel: +41/(0)22 816 19-42     //|                 3, Chemin du Pré-Fleuri
                             _// | bilis Systems   CH-1228 Plan-les-Ouates
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/