All of lore.kernel.org
 help / color / mirror / Atom feed
From: b29396@freescale.com (Dong Aisheng)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/7] mfd: add imx syscon driver based on regmap
Date: Thu, 23 Aug 2012 15:06:58 +0800	[thread overview]
Message-ID: <20120823070657.GE10057@shlinux2.ap.freescale.net> (raw)
In-Reply-To: <5035BCB1.8000801@wwwdotorg.org>

On Thu, Aug 23, 2012 at 01:16:33PM +0800, Stephen Warren wrote:
> On 08/22/2012 04:57 AM, Dong Aisheng wrote:
> > On Wed, Aug 22, 2012 at 04:29:41PM +0800, Zhao Richard-B20223 wrote:
> >> On Wed, Aug 22, 2012 at 03:18:42PM +0800, Dong Aisheng wrote:
> >>> Add regmap based imx syscon driver.
> >>> This is usually used for access misc bits in registers which does not belong
> >>> to a specific module, for example, IOMUXC GPR and ANATOP.
> >>> With this driver, we provide a standard API for client driver to call to
> >>> access registers which are registered into syscon.
> 
> >>> +static int imx_syscon_match(struct device *dev, void *data)
> >>> +{
> >>> +	struct imx_syscon *syscon = dev_get_drvdata(dev);
> >>> +	struct device_node *dn = data;
> >>> +
> >>> +	return (syscon->dev->of_node == dn) ? 1 : 0;
> >>> +}
> >>> +
> >>> +int imx_syscon_write(struct device_node *np, u32 reg, u32 val)
> >>
> >> For API function, is it better to use struct device rather not np?
> >>  - it won't need to search dev in below code every time it access
> >>    registers.
> >
> > The purpose is not require client driver to know the implementation details
> > of imx_syscon_{read/write} API, it's more easy to use since client only
> > needs pass the device node to which it wants to read/write.
> > 
> > For search dev, it doesn't look like a big issue since it only search devices
> > attached on the driver which is very quick.
> > And hide it in common API does not require every client driver to write
> > duplicated codes.
> 
> You could still implement a function:
> 
> struct device *imx_syscon_lookup(struct device_node *np)
> 
> ... and require all clients to call that, and pass the dev to the other
> functions. That'd still keep all the lookup code in one place, but
> prevent it having to run every time, no matter how small it is.
> 
> I think such an API is required anyway, since client drivers need some
> way to determine whether the imx_syscon driver is available yet, and if
> not defer their probe until it is.
> 
> So, clients would do:
> 
> foo->syscon_dev = imx_syscon_lookup(np);
> if (!foo->syscon_dev)
>     return -EPROBE_DEFER;
> 
> rather than:
> 
> foo->syscon_np = np;
> 
> Not too much overhead/boiler-plate in each client driver.
> 
Looks like a good idea.

Regards
Dong Aisheng

WARNING: multiple messages have this Message-ID (diff)
From: Dong Aisheng <b29396-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
To: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
Cc: Zhao Richard-B20223
	<B20223-KZfg59tc24xl57MIdRCFDg@public.gmane.org>,
	"linus.walleij-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org"
	<linus.walleij-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org>,
	"devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org"
	<devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org>,
	"s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org"
	<s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>,
	"broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org"
	<broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>,
	"linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org"
	<rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>,
	"kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org"
	<kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>,
	"paul.liu-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org"
	<paul.liu-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	"lrg-l0cyMroinI0@public.gmane.org"
	<lrg-l0cyMroinI0@public.gmane.org>,
	"linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org"
	<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>,
	"sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org"
	<sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Subject: Re: [PATCH 1/7] mfd: add imx syscon driver based on regmap
Date: Thu, 23 Aug 2012 15:06:58 +0800	[thread overview]
Message-ID: <20120823070657.GE10057@shlinux2.ap.freescale.net> (raw)
In-Reply-To: <5035BCB1.8000801-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>

On Thu, Aug 23, 2012 at 01:16:33PM +0800, Stephen Warren wrote:
> On 08/22/2012 04:57 AM, Dong Aisheng wrote:
> > On Wed, Aug 22, 2012 at 04:29:41PM +0800, Zhao Richard-B20223 wrote:
> >> On Wed, Aug 22, 2012 at 03:18:42PM +0800, Dong Aisheng wrote:
> >>> Add regmap based imx syscon driver.
> >>> This is usually used for access misc bits in registers which does not belong
> >>> to a specific module, for example, IOMUXC GPR and ANATOP.
> >>> With this driver, we provide a standard API for client driver to call to
> >>> access registers which are registered into syscon.
> 
> >>> +static int imx_syscon_match(struct device *dev, void *data)
> >>> +{
> >>> +	struct imx_syscon *syscon = dev_get_drvdata(dev);
> >>> +	struct device_node *dn = data;
> >>> +
> >>> +	return (syscon->dev->of_node == dn) ? 1 : 0;
> >>> +}
> >>> +
> >>> +int imx_syscon_write(struct device_node *np, u32 reg, u32 val)
> >>
> >> For API function, is it better to use struct device rather not np?
> >>  - it won't need to search dev in below code every time it access
> >>    registers.
> >
> > The purpose is not require client driver to know the implementation details
> > of imx_syscon_{read/write} API, it's more easy to use since client only
> > needs pass the device node to which it wants to read/write.
> > 
> > For search dev, it doesn't look like a big issue since it only search devices
> > attached on the driver which is very quick.
> > And hide it in common API does not require every client driver to write
> > duplicated codes.
> 
> You could still implement a function:
> 
> struct device *imx_syscon_lookup(struct device_node *np)
> 
> ... and require all clients to call that, and pass the dev to the other
> functions. That'd still keep all the lookup code in one place, but
> prevent it having to run every time, no matter how small it is.
> 
> I think such an API is required anyway, since client drivers need some
> way to determine whether the imx_syscon driver is available yet, and if
> not defer their probe until it is.
> 
> So, clients would do:
> 
> foo->syscon_dev = imx_syscon_lookup(np);
> if (!foo->syscon_dev)
>     return -EPROBE_DEFER;
> 
> rather than:
> 
> foo->syscon_np = np;
> 
> Not too much overhead/boiler-plate in each client driver.
> 
Looks like a good idea.

Regards
Dong Aisheng

WARNING: multiple messages have this Message-ID (diff)
From: Dong Aisheng <b29396@freescale.com>
To: Stephen Warren <swarren@wwwdotorg.org>
Cc: Zhao Richard-B20223 <B20223@freescale.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"linus.walleij@stericsson.com" <linus.walleij@stericsson.com>,
	"s.hauer@pengutronix.de" <s.hauer@pengutronix.de>,
	"shawn.guo@linaro.org" <shawn.guo@linaro.org>,
	"kernel@pengutronix.de" <kernel@pengutronix.de>,
	"grant.likely@secretlab.ca" <grant.likely@secretlab.ca>,
	"rob.herring@calxeda.com" <rob.herring@calxeda.com>,
	"sameo@linux.intel.com" <sameo@linux.intel.com>,
	"lrg@ti.com" <lrg@ti.com>,
	"broonie@opensource.wolfsonmicro.com" 
	<broonie@opensource.wolfsonmicro.com>,
	"devicetree-discuss@lists.ozlabs.org" 
	<devicetree-discuss@lists.ozlabs.org>,
	"paul.liu@linaro.org" <paul.liu@linaro.org>
Subject: Re: [PATCH 1/7] mfd: add imx syscon driver based on regmap
Date: Thu, 23 Aug 2012 15:06:58 +0800	[thread overview]
Message-ID: <20120823070657.GE10057@shlinux2.ap.freescale.net> (raw)
In-Reply-To: <5035BCB1.8000801@wwwdotorg.org>

On Thu, Aug 23, 2012 at 01:16:33PM +0800, Stephen Warren wrote:
> On 08/22/2012 04:57 AM, Dong Aisheng wrote:
> > On Wed, Aug 22, 2012 at 04:29:41PM +0800, Zhao Richard-B20223 wrote:
> >> On Wed, Aug 22, 2012 at 03:18:42PM +0800, Dong Aisheng wrote:
> >>> Add regmap based imx syscon driver.
> >>> This is usually used for access misc bits in registers which does not belong
> >>> to a specific module, for example, IOMUXC GPR and ANATOP.
> >>> With this driver, we provide a standard API for client driver to call to
> >>> access registers which are registered into syscon.
> 
> >>> +static int imx_syscon_match(struct device *dev, void *data)
> >>> +{
> >>> +	struct imx_syscon *syscon = dev_get_drvdata(dev);
> >>> +	struct device_node *dn = data;
> >>> +
> >>> +	return (syscon->dev->of_node == dn) ? 1 : 0;
> >>> +}
> >>> +
> >>> +int imx_syscon_write(struct device_node *np, u32 reg, u32 val)
> >>
> >> For API function, is it better to use struct device rather not np?
> >>  - it won't need to search dev in below code every time it access
> >>    registers.
> >
> > The purpose is not require client driver to know the implementation details
> > of imx_syscon_{read/write} API, it's more easy to use since client only
> > needs pass the device node to which it wants to read/write.
> > 
> > For search dev, it doesn't look like a big issue since it only search devices
> > attached on the driver which is very quick.
> > And hide it in common API does not require every client driver to write
> > duplicated codes.
> 
> You could still implement a function:
> 
> struct device *imx_syscon_lookup(struct device_node *np)
> 
> ... and require all clients to call that, and pass the dev to the other
> functions. That'd still keep all the lookup code in one place, but
> prevent it having to run every time, no matter how small it is.
> 
> I think such an API is required anyway, since client drivers need some
> way to determine whether the imx_syscon driver is available yet, and if
> not defer their probe until it is.
> 
> So, clients would do:
> 
> foo->syscon_dev = imx_syscon_lookup(np);
> if (!foo->syscon_dev)
>     return -EPROBE_DEFER;
> 
> rather than:
> 
> foo->syscon_np = np;
> 
> Not too much overhead/boiler-plate in each client driver.
> 
Looks like a good idea.

Regards
Dong Aisheng


  parent reply	other threads:[~2012-08-23  7:06 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-22  7:18 [PATCH 0/7] add imx-syscon driver for general registers access Dong Aisheng
2012-08-22  7:18 ` Dong Aisheng
2012-08-22  7:18 ` Dong Aisheng
2012-08-22  7:18 ` [PATCH 1/7] mfd: add imx syscon driver based on regmap Dong Aisheng
2012-08-22  7:18   ` Dong Aisheng
2012-08-22  7:18   ` Dong Aisheng
2012-08-22  8:29   ` Richard Zhao
2012-08-22  8:29     ` Richard Zhao
2012-08-22  8:29     ` Richard Zhao
2012-08-22 10:57     ` Dong Aisheng
2012-08-22 10:57       ` Dong Aisheng
2012-08-23  5:16       ` Stephen Warren
2012-08-23  5:16         ` Stephen Warren
2012-08-23  6:09         ` Richard Zhao
2012-08-23  6:09           ` Richard Zhao
2012-08-23  6:09           ` Richard Zhao
2012-08-23  7:06         ` Dong Aisheng [this message]
2012-08-23  7:06           ` Dong Aisheng
2012-08-23  7:06           ` Dong Aisheng
2012-08-22 16:02   ` Mark Brown
2012-08-22 16:02     ` Mark Brown
2012-08-22 16:02     ` Mark Brown
2012-08-23  7:26     ` Dong Aisheng
2012-08-23  7:26       ` Dong Aisheng
2012-08-23 11:06       ` Mark Brown
2012-08-23 11:06         ` Mark Brown
2012-08-23 11:06         ` Mark Brown
2012-08-24  2:28         ` Dong Aisheng
2012-08-24  2:28           ` Dong Aisheng
2012-08-24  2:28           ` Dong Aisheng
2012-08-24  6:43   ` Shawn Guo
2012-08-24  6:43     ` Shawn Guo
2012-08-24  6:43     ` Shawn Guo
2012-08-22  7:18 ` [PATCH 2/7] ARM: imx6q: add iomuxc gpr support into imx-syscon Dong Aisheng
2012-08-22  7:18   ` Dong Aisheng
2012-08-22  7:18   ` Dong Aisheng
2012-08-22  7:18 ` [PATCH 3/7] ARM: imx6q: add anatop " Dong Aisheng
2012-08-22  7:18   ` Dong Aisheng
2012-08-22  7:18   ` Dong Aisheng
2012-08-22  7:18 ` [PATCH 4/7] regulator: anatop-regulator: convert to use imx-syscon to access anatop register Dong Aisheng
2012-08-22  7:18   ` Dong Aisheng
2012-08-22  7:18   ` Dong Aisheng
2012-08-22 15:59   ` Mark Brown
2012-08-22 15:59     ` Mark Brown
2012-08-23  7:15     ` Dong Aisheng
2012-08-23  7:15       ` Dong Aisheng
2012-08-23  7:15       ` Dong Aisheng
2012-08-23 11:17       ` Mark Brown
2012-08-23 11:17         ` Mark Brown
2012-08-24  2:29         ` Dong Aisheng
2012-08-24  2:29           ` Dong Aisheng
2012-08-23  5:21   ` Stephen Warren
2012-08-23  5:21     ` Stephen Warren
2012-08-23  6:12     ` Richard Zhao
2012-08-23  6:12       ` Richard Zhao
2012-08-23  6:12       ` Richard Zhao
2012-08-23 17:56       ` Stephen Warren
2012-08-23 17:56         ` Stephen Warren
2012-08-24  2:37         ` Dong Aisheng
2012-08-24  2:37           ` Dong Aisheng
2012-08-24  2:37           ` Dong Aisheng
2012-08-23  7:32     ` Dong Aisheng
2012-08-23  7:32       ` Dong Aisheng
2012-08-23  7:32       ` Dong Aisheng
2012-08-22  7:18 ` [PATCH 5/7] ARM: imx6q: convert to use imx-syscon to access anatop registers Dong Aisheng
2012-08-22  7:18   ` Dong Aisheng
2012-08-22  7:18   ` Dong Aisheng
2012-08-22  7:18 ` [PATCH 6/7] ARM: dts: imx6q: add simple-bus compatible string for anatop Dong Aisheng
2012-08-22  7:18   ` Dong Aisheng
2012-08-22  7:18   ` Dong Aisheng
2012-08-22  8:52   ` Richard Zhao
2012-08-22  8:52     ` Richard Zhao
2012-08-22  8:52     ` Richard Zhao
2012-08-22 11:02     ` Dong Aisheng
2012-08-22 11:02       ` Dong Aisheng
2012-08-22  7:18 ` [PATCH 7/7] mfd: anatop-mfd: remove anatop driver Dong Aisheng
2012-08-22  7:18   ` Dong Aisheng
2012-08-22  7:18   ` Dong Aisheng

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=20120823070657.GE10057@shlinux2.ap.freescale.net \
    --to=b29396@freescale.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.