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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <50B5D161.6010200@ti.com>
Date:	Wed, 28 Nov 2012 09:54:57 +0100
From:	Peter Ujfalusi <peter.ujfalusi@...com>
To:	Grant Likely <grant.likely@...retlab.ca>,
	Lars-Peter Clausen <lars@...afoo.de>,
	Thierry Reding <thierry.reding@...onic-design.de>
CC:	Linus Walleij <linus.walleij@...aro.org>,
	Rob Landley <rob@...dley.net>,
	<devicetree-discuss@...ts.ozlabs.org>, <linux-doc@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	<linux-omap@...r.kernel.org>
Subject: Re: [PATCH] gpio: New driver for GPO emulation using PWM generators

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/