Messages in this thread | | | Date | Mon, 7 Mar 2011 10:35:47 -0600 | Subject | Re: [PWM v6 1/3] PWM: Implement a generic PWM framework | From | Bill Gatliff <> |
| |
Jack:
On Sun, Mar 6, 2011 at 10:20 PM, Jack Stone <jwjstone@fastmail.fm> wrote: > On 06/03/2011 04:17, Bill Gatliff wrote: >> +The Generic PWM Device API framework is implemented in >> +include/linux/pwm/pwm.h and drivers/pwm/pwm.c. The functions therein >> +use information from pwm_device and pwm__config structures to invoke > > Could you fix that to be pwm_config please.
Fixed.
>> +menuconfig GENERIC_PWM >> + tristate "PWM Support" >> + help >> + Enables PWM device support implemented via a generic >> + framework. If unsure, say N. >> + > > Does this need help text? Can't we just select GENERIC_PWM for the > drivers / users that need it?
That would be my preference, actually. I'll see if I can do that at the next review.
>> + >> + if (p->ops->request) { >> + ret = p->ops->request(p); >> + if (ret) { >> + clear_bit(FLAG_REQUESTED, &p->flags); >> + goto done; > > You don't need this goto here.
Fixed.
>> +struct pwm_device *pwm_request(const char *bus_id, int id, const char *label) >> +{ >> + char name[256]; >> + int ret; >> + >> + if (id == -1) >> + ret = scnprintf(name, sizeof name, "%s", bus_id); > > Kernel doc comment for the function to describe the id == -1 case?
Fixed.
>> +static void pwm_handler(struct work_struct *w) >> +{ >> + struct pwm_device *p = container_of(w, struct pwm_device, >> + handler_work); > > You could just not queue the work if !p->handler
Good point. Fixed.
>> + if (id == -1) >> + ret = scnprintf(name, sizeof name, "%s", dev_name(parent)); > > A kernel doc comment to explain id == -1 would be nice. >
Fixed.
> Very nice framework!
Thank you!!
> > Would be nice to have kernel doc comments for the functions but that can > follow on later.
I'll try to do some of it for the next review series.
b.g. -- Bill Gatliff bgat@billgatliff.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |