Re: [PATCH] gpio: New driver for GPO emulation using PWM generators

[Peter Ujfalusi <peter.ujfalusi-l0cyMroinI0@public.gmane.org>]

Hi Grant, Lars, Thierry,

On 11/26/2012 04:46 PM, Grant Likely wrote:
> You're effectively asking the pwm layer to behave like a gpio (which
> is completely reasonable). Having a completely separate translation node
> really doesn't make sense because it is entirely a software construct.
> In fact, the way your using it is *entirely* to make the Linux driver
> model instantiate the translation code. It has *nothing* to do with the
> structure of the hardware. It makes complete sense that if a PWM is
> going to be used as a GPIO, then the PWM node should conform to the GPIO
> binding.

I understand your point around this. I might say I agree with it as well...
I spent yesterday with prototyping and I'm not really convinced that it is a
good approach from C code point of view. I got it working, yes.
In essence this is what I have on top of the slightly modified gpio-pwm.c
driver I have submitted:

DTS files:
twl_pwm: pwm {
	/* provides two PWMs (id 0, 1 for PWM1 and PWM2) */
	compatible = "ti,twl6030-pwm";
	#pwm-cells = <2>;

	/* Enable GPIO us of the PWMs */
	gpio-controller = <1>;
	#gpio-cells = <2>;
	pwm,period_ns = <7812500>;
};

leds {
	compatible = "gpio-leds";
	backlight {
		label = "omap4::backlight";
		gpios = <&twl_pwm 1 0>; /* PWM1 of twl6030 */
	};

	keypad {
		label = "omap4::keypad";
		gpios = <&twl_pwm 0 0>; /* PWM0 of twl6030 */
	};
};

The bulk of the code in drivers/pwm/core.c to create the pwm-gpo device when
it is requested going to look something like this. I have removed the error
checks for now and I still don't have the code to clean up the allocated
memory for the created device on error, or in case the module is unloaded. We
should also prevent the pwm core from removal when the pwm-gpo driver is loaded.
We need to create the platform device for gpo-pwm, create the pdata structure
for it and fill it in. We also need to hand craft the pwm_lookup table so we
can use pwm_get() to request the PWM. I have other minor changes around this
to get things working when we booted with DT.
So the function to do the heavy lifting is something like this:
static void of_pwmchip_as_gpio(struct pwm_chip *chip)
{
	struct platform_device *pdev;
	struct gpio_pwm *gpos;
	struct gpio_pwm_pdata *pdata;
	struct pwm_lookup *lookup;
	char gpodev_name[15];
	int i;
	u32 gpio_mode = 0;
	u32 period_ns = 0;

	of_property_read_u32(chip->dev->of_node, "gpio-controller",
			     &gpio_mode);
	if (!gpio_mode)
		return;

	of_property_read_u32(chip->dev->of_node, "pwm,period_ns", &period_ns);
	if (!period_ns) {
		dev_err(chip->dev,
			"period_ns is not specified for GPIO use\n");
		return;
	}

	lookup = devm_kzalloc(chip->dev, sizeof(*lookup) * chip->npwm,
			      GFP_KERNEL);
	pdata = devm_kzalloc(chip->dev, sizeof(*pdata), GFP_KERNEL);
	gpos = devm_kzalloc(chip->dev, sizeof(*gpos) * chip->npwm,
			    GFP_KERNEL);

	pdata->gpos = gpos;
	pdata->num_gpos = chip->npwm;
	pdata->gpio_base = -1;

	pdev = platform_device_alloc("pwm-gpo", chip->base);
	pdev->dev.parent = chip->dev;

	sprintf(gpodev_name, "pwm-gpo.%d", chip->base);
	for (i = 0; i < chip->npwm; i++) {
		struct gpio_pwm *gpo = &gpos[i];
		struct pwm_lookup *pl = &lookup[i];
		char con_id[15];

		sprintf(con_id, "pwm-gpo.%d", chip->base + i);

		/* prepare GPO information */
		gpo->pwm_period_ns = period_ns;
		gpo->name = kmemdup(con_id, sizeof(con_id), GFP_KERNEL);;

		/* prepare pwm lookup table */
		pl->provider = dev_name(chip->dev);
		pl->index = i;
		pl->dev_id = kmemdup(gpodev_name, sizeof(gpodev_name),
				     GFP_KERNEL);
		pl->con_id = kmemdup(con_id, sizeof(con_id), GFP_KERNEL);
	}

	platform_device_add_data(pdev, pdata, sizeof(*pdata));
	pwm_add_table(lookup, chip->npwm);
	platform_device_add(pdev);
}

PS: as I have said I have removed the error check just to make the code
snippet more readable and yes we need to do some memory cleanup as well at the
right time.

Is this something you would like to see?

-- 
Péter