public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: ryan@bluewatersys•com (Ryan Mallon)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 1/7] pwm: Add pwm core driver
Date: Wed, 29 Sep 2010 08:42:05 +1300	[thread overview]
Message-ID: <4CA2450D.8090104@bluewatersys.com> (raw)
In-Reply-To: <1285659648-21409-2-git-send-email-arun.murthy@stericsson.com>

On 09/28/2010 08:40 PM, Arun Murthy wrote:
> The existing pwm based led and backlight driver makes use of the
> pwm(include/linux/pwm.h). So all the board specific pwm drivers will
> be exposing the same set of function name as in include/linux/pwm.h.
> As a result build fails.
> 
> In order to overcome this issue all the pwm drivers must register to
> some core pwm driver with function pointers for pwm operations (i.e
> pwm_config, pwm_enable, pwm_disable).

The other major benefit of this patch set is that it allows non-soc
pwms, such as those provided on an i2c or spi device, to be added easily.

> The clients of pwm device will have to call pwm_request, wherein
> they will get the pointer to struct pwm_ops. This structure include
> function pointers for pwm_config, pwm_enable and pwm_disable.
> 
> Signed-off-by: Arun Murthy <arun.murthy@stericsson•com>
> Acked-by: Linus Walleij <linus.walleij@stericsson•com>
> ---

> +menuconfig PWM_DEVICES
> +	bool "PWM devices"
> +	default y
> +	---help---
> +	  Say Y here to get to see options for device drivers from various
> +	  different categories. This option alone does not add any kernel code.

This help text doesn't really explain what the option does.

> +struct pwm_device {
> +	struct pwm_ops *pops;
> +	int pwm_id;
> +};
> +
> +struct pwm_dev_info {
> +	struct pwm_device *pwm_dev;
> +	struct list_head list;
> +};

These two structures can be merged into one which will make the code
much simpler.

> +static struct pwm_dev_info *di;

This appears to be used as just a list, and could be replaced by:

static LIST_HEAD(pwm_list);

> +struct pwm_device *pwm_request(int pwm_id, const char *name)
> +{
> +	struct pwm_dev_info *pwm;
> +	struct list_head *pos;
> +
> +	down_read(&pwm_list_lock);
> +	list_for_each(pos, &di->list) {
> +		pwm = list_entry(pos, struct pwm_dev_info, list);
> +		if ((!strcmp(pwm->pwm_dev->pops->name, name)) &&
> +				(pwm->pwm_dev->pwm_id == pwm_id)) {
> +			up_read(&pwm_list_lock);
> +			return pwm->pwm_dev;
> +		}
> +	}
> +	up_read(&pwm_list_lock);
> +	return ERR_PTR(-ENOENT);
> +}

Is it by design that multiple users can request and use the same pwm or
should pwms have a use count and return -EBUSY if already requested?

> +static int __init pwm_init(void)
> +{
> +	struct pwm_dev_info *pwm;
> +
> +	pwm = kzalloc(sizeof(struct pwm_dev_info), GFP_KERNEL);
> +	if (!pwm)
> +		return -ENOMEM;
> +	INIT_LIST_HEAD(&pwm->list);
> +	di = pwm;
> +	return 0;

If di is changed to a list as suggested above this function does not
need to exist.

> +static void __exit pwm_exit(void)
> +{
> +	kfree(di);

Do you need to ensure the list is empty first or do module dependencies
ensure that?

~Ryan

-- 
Bluewater Systems Ltd - ARM Technology Solution Centre

Ryan Mallon         		5 Amuri Park, 404 Barbadoes St
ryan at bluewatersys.com         	PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com	New Zealand
Phone: +64 3 3779127		Freecall: Australia 1800 148 751
Fax:   +64 3 3779135			  USA 1800 261 2934

  parent reply	other threads:[~2010-09-28 19:42 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-28  7:40 [PATCHv2 0/7] PWM core driver for pwm based led and backlight driver Arun Murthy
2010-09-28  7:40 ` [PATCH 1/7] pwm: Add pwm core driver Arun Murthy
2010-09-28  8:14   ` Vasily Khoruzhick
     [not found]     ` <F45880696056844FA6A73F415B568C69532DC2FA8F@EXDCVYMBSTM006.EQ1STM.local>
2010-09-28  8:47       ` Vasily Khoruzhick
     [not found]       ` <F45880696056844FA6A73F415B568C69532DC2FA8F@EXDCVYMBSTM006.EQ1STM.loca l>
2010-09-28  8:50         ` Hemanth V
     [not found]           ` <F45880696056844FA6A73F415B568C69532DC2FB21@EXDCVYMBSTM006.EQ1STM.local>
     [not found]             ` <F45880696056844FA6A73F415B568C69532DC2FB21@EXDCVYMBSTM006.EQ1STM.loca l>
2010-09-28  9:34               ` Hemanth V
     [not found]                 ` <F45880696056844FA6A73F415B568C69532DC2FBF9@EXDCVYMBSTM006.EQ1STM.local>
     [not found]                   ` <F45880696056844FA6A73F415B568C69532DC2FBF9@EXDCVYMBSTM006.EQ1STM.loca l>
2010-09-28 10:41                     ` Hemanth V
2010-09-28  8:54   ` Lars-Peter Clausen
     [not found]     ` <F45880696056844FA6A73F415B568C69532DC2FB6B@EXDCVYMBSTM006.EQ1STM.local>
2010-09-28  9:57       ` Lars-Peter Clausen
     [not found]         ` <F45880696056844FA6A73F415B568C69532DC2FC60@EXDCVYMBSTM006.EQ1STM.local>
2010-09-28 21:04           ` Lars-Peter Clausen
2010-09-29  4:49             ` Arun MURTHY
2010-09-29 12:12               ` Trilok Soni
2010-10-01  3:25                 ` Arun MURTHY
2010-10-01  6:47                   ` Trilok Soni
2010-10-01  7:25                     ` Arun MURTHY
2010-10-01  7:42                       ` Jassi Brar
2010-10-01  8:46                         ` Arun MURTHY
2010-10-01 10:39                           ` Jassi Brar
2010-10-01 18:00                           ` Mark Brown
2010-10-04  4:22                             ` Arun MURTHY
2010-09-28 17:46   ` Mark Brown
2010-09-28 19:42   ` Ryan Mallon [this message]
2010-09-28  7:40 ` [PATCH 2/7] backlight:pwm: add an element 'name' to platform data Arun Murthy
2010-09-28 17:47   ` Mark Brown
2010-09-28  7:40 ` [PATCH 3/7] leds: pwm: add a new " Arun Murthy
2010-09-28  8:01   ` Eric Miao
2010-09-28  8:36     ` Arun MURTHY
2010-09-28  7:40 ` [PATCH 4/7] pwm: Align existing pwm drivers with pwm-core driver Arun Murthy
2010-09-28  8:58   ` Lars-Peter Clausen
     [not found]     ` <F45880696056844FA6A73F415B568C69532DC2FB98@EXDCVYMBSTM006.EQ1STM.local>
2010-09-28 10:10       ` Lars-Peter Clausen
2010-09-28  7:40 ` [PATCH 5/7] platform: Update the pwm based led and backlight platform data Arun Murthy
2010-09-28  7:40 ` [PATCH 7/7] pwm: Modify backlight and led Kconfig aligning to pwm core Arun Murthy
     [not found] ` <1285659648-21409-7-git-send-email-arun.murthy@stericsson.com>
2010-09-28  8:02   ` [PATCH 6/7] pwm: move existing pwm driver to drivers/pwm Eric Miao
  -- strict thread matches above, loose matches on Subject: below --
2010-09-28 10:35 [PATCH 0/7] PWM core driver for pwm based led and backlight driver Arun Murthy
2010-09-28 10:35 ` [PATCH 1/7] pwm: Add pwm core driver Arun Murthy
2010-09-28 12:53   ` Hemanth V
2010-09-28 13:06     ` Samuel Ortiz
2010-09-28 13:35       ` Felipe Balbi

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=4CA2450D.8090104@bluewatersys.com \
    --to=ryan@bluewatersys$(echo .)com \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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