Mike:
Thanks for looking at this code! Replies inlined below.
On Sat, Feb 12, 2011 at 8:53 PM, Mike Frysinger <vapier.adi@xxxxxxxxx> wrote:
> document the module name in the help ?
Not sure what you are asking. Like this?
config GPIO_PWM
tristate "GPIO+hrtimer PWM device emulation"
depends on GENERIC_PWM
help
When enabled, this option creates a module named gpio-pwm
that emulates single-channel PWM devices using
high-resolution timers and GPIO pins. The PWM framework
allows you to create as many of these devices as desired,
subject to CPU overhead and GPIO pin availability. If
unsure, say N.
>
>> --- /dev/null
>> +++ b/drivers/pwm/gpio-pwm.c
>> +#define DRIVER_NAME "gpio-pwm"
>
> use KBUILD_MODNAME instead ?
Good suggestion. Fixed.
>
>> +static void gpio_pwm_work (struct work_struct *work)
>
> no space before the "("
Fixed.
>> +static struct pwm_device_ops gpio_pwm_device_ops = {
>> + .config = gpio_pwm_config,
>> + .config_nosleep = gpio_pwm_config_nosleep,
>> + .request = gpio_pwm_request,
>> +};
>
> is this struct not constified ? same for some of the other structs in
> this file ...
It isn't constified, but it should be. But if I do, I get lots of
"discards qualifiers" warnings because const isn't used in the
functions I pass these structures to. So I kind of have to leave it
as-is, no?
>
>> + gp = kzalloc(sizeof(*gp), GFP_KERNEL);
>> + if (IS_ERR_OR_NULL(gp))
>
> i dont think the kmalloc funcs return a pointer error. it's either
> NULL or it's a valid pointer. this comes up a few times ...
Ok. I like that macro so much, sometimes I get carried away. :)
Fixed. (I found only two locations where this mistake occurred).
>
>> +EXPORT_SYMBOL(gpio_pwm_create);
>> +EXPORT_SYMBOL(gpio_pwm_destroy);
>
> are people expected to call these directly ?
Yes. It's equivalent to doing a platform_device_register_simple(),
except that gpio_pwm devices aren't platform devices--- they are just
a combination of an hrtimer and a gpio pin.
> is that useful if they cant call any of the config funcs ?
Users of gpio_pwm aren't supposed to call the config functions in
gpio-pwm.c, they are supposed to invoke them indirectly via the
regular PWM API (pwm.c).
b.g.
--
Bill Gatliff
bgat@xxxxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
