public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Olliver Schinagl <oliver+list@schinagl.nl>
To: Thierry Reding <thierry.reding@gmail.com>
Cc: linux-pwm@vger.kernel.org,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: [RFC] pwm: core: unsigned or signed ints for pwm_config
Date: Tue, 29 Sep 2015 09:19:27 +0200	[thread overview]
Message-ID: <560A3B7F.30803@schinagl.nl> (raw)

Hey Thierry, list

I'm going over the pwm core and notice that in the pwm header, duty_ns 
and period_ns is internally stored as an unsigned int.

struct pwm_device {
     const char *label;
     unsigned long flags;
     unsigned int hwpwm;
     unsigned int pwm;
     struct pwm_chip *chip;
     void *chip_data;

     unsigned int period;
     unsigned int duty_cycle;
     enum pwm_polarity polarity;
};

However, pwm_config takes signed ints
int pwm_config(struct pwm_device *pwm, int duty_ns, int period_ns);

So digging a little deeper in the PWM core, I see that pwm_config 
dissallows negative ints, so having them unsigned has no benefit (and 
technically is illegal)
     if (!pwm  || duty_ns < 0|| period_ns= 0 || duty_ns > period_ns)
         return -EINVAL;

and because (after the check) we cram the signed int into an unsigned one:

     pwm->duty_cycle = duty_ns;
     pwm->period = period_ns;

This could end up badly when using any unsigned int larger then INT_MAX 
and thus ending up with a negative duty/period. I haven't checked deeper 
if this is accounted for later, but would it be worth my time to convert 
all ints to unsigned ints? Since negative period and duty cycles are 
really not possible anyway?

Olliver

             reply	other threads:[~2015-09-29  7:19 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-29  7:19 Olliver Schinagl [this message]
2015-09-29  7:45 ` [RFC] pwm: core: unsigned or signed ints for pwm_config Thierry Reding
2015-10-01 18:46   ` Olliver Schinagl
2015-10-05 10:00     ` Thierry Reding

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=560A3B7F.30803@schinagl.nl \
    --to=oliver+list@schinagl.nl \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=thierry.reding@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox