Hi Grant,
On 11/23/2012 08:55 AM, Grant Likely wrote:
> Ugh. and this is why I wanted the PWM and GPIO subsystems to use the
> same namespace and binding. <grumble, mutter> But that's not your fault.
>
> It's pretty horrible to have a separate translator node to convert a PWM
> into a GPIO (with output only of course). The gpio properties should
> appear directly in the PWM node itself and the translation code should
> be in either the pwm or the gpio core. I don't think it should look like
> a separate device.
Let me see if I understand your suggestion correctly. In the DT you suggest
something like this:
twl_pwmled: pwmled {
compatible = "ti,twl4030-pwmled";
#pwm-cells = <2>;
#gpio-cells = <2>;
gpio-controller;
};
led_user {
compatible = "pwm-leds";
pwms = <&twl_pwmled 1 7812500>; /* PWMB/LEDB from twl4030 */
pwm-names = "PMU_STAT LED";
label = "beagleboard::pmu_stat";
max-brightness = <127>;
};
vdd_usbhost: fixedregulator-vdd-usbhost {
compatible = "regulator-fixed";
regulator-name = "USBHOST_POWER";
regulator-min-microvolt = <5000000>;
regulator-max-microvolt = <5000000>;
gpio = <&twl_pwmled 0 7812500>; /* PWMA/LEDA from twl4030 */
enable-active-high;
regulator-boot-on;
};
With this I think this is what should happen in code level:
- the "pwm-gpo" driver does not have of_match_table at all.
- the driver for the "ti,twl4030-pwmled" is loaded.
- it prepares and calls pwmchip_add() to add the PWM chip.
- the of_pwmchip_add() will look for gpio-controller property of the node
- if it is found it prepares the pdata (based on the PWM chip information)
for the "pwm-gpo" driver and registers the platform_device for it.
- the "pwm-gpo" driver will use:
priv->gpio_chip.of_node = pdev->dev.parent->of_node;
In DT boot we are fine with this I think.
When it comes to legacy boot (boot without DT) I think we should still have
the two layers to avoid big changes which would affect all existing pwm
drivers. Something like this in the board files:
static struct pwm_lookup pwm_lookup[] = {
/* LEDA -> nUSBHOST_PWR_EN */
PWM_LOOKUP("twl-pwmled", 0, "pwm-gpo", "nUSBHOST_PWR_EN"),
/* LEDB -> PMU_STAT */
PWM_LOOKUP("twl-pwmled", 1, "leds_pwm", "beagleboard::pmu_stat"),
};
/* for the LED user of PWM */
static struct led_pwm pwm_leds[] = {
{
.name = "beagleboard::pmu_stat",
.max_brightness = 127,
.pwm_period_ns = 7812500,
},
};
static struct led_pwm_platform_data pwm_data = {
.num_leds = ARRAY_SIZE(pwm_leds),
.leds = pwm_leds,
};
static struct platform_device leds_pwm = {
.name = "leds_pwm",
.id = -1,
.dev = {
.platform_data = &pwm_data,
},
};
/* for the GPIO user of PWM */
static struct gpio_pwm pwm_gpios[] = {
{
.name = "nUSBHOST_PWR_EN",
.pwm_period_ns = 7812500,
},
};
static struct gpio_pwm_pdata pwm_gpio_data = {
.num_gpos = ARRAY_SIZE(pwm_gpios),
.gpos = pwm_gpios,
.setup = beagle_pwm_gpio_setup, /*to get the gpio base */
};
static struct platform_device gpos_pwm = {
.name = "pwm-gpo",
.id = -1,
.dev = {
.platform_data = &pwm_gpio_data,
},
};
static int beagle_pwm_gpio_setup(struct device *dev, unsigned gpio,
unsigned ngpio)
{
beagle_usbhub_pdata.gpio = gpio; /* fixed_voltage_config struct */
platform_device_register(&beagle_usbhub);
return 0;
}
What do you think?
--
Péter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
