linux-kernel - Re: [PATCH 1/7] mfd: add imx syscon driver based on regmap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120823060937.GL4011@b20223-02.ap.freescale.net>
Date:	Thu, 23 Aug 2012 14:09:38 +0800
From:	Richard Zhao <richard.zhao@...escale.com>
To:	Stephen Warren <swarren@...dotorg.org>
CC:	Dong Aisheng <b29396@...escale.com>,
	Zhao Richard-B20223 <B20223@...escale.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linus.walleij@...ricsson.com" <linus.walleij@...ricsson.com>,
	"s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
	"shawn.guo@...aro.org" <shawn.guo@...aro.org>,
	"kernel@...gutronix.de" <kernel@...gutronix.de>,
	"grant.likely@...retlab.ca" <grant.likely@...retlab.ca>,
	"rob.herring@...xeda.com" <rob.herring@...xeda.com>,
	"sameo@...ux.intel.com" <sameo@...ux.intel.com>,
	"lrg@...com" <lrg@...com>,
	"broonie@...nsource.wolfsonmicro.com" 
	<broonie@...nsource.wolfsonmicro.com>,
	"devicetree-discuss@...ts.ozlabs.org" 
	<devicetree-discuss@...ts.ozlabs.org>,
	"paul.liu@...aro.org" <paul.liu@...aro.org>
Subject: Re: [PATCH 1/7] mfd: add imx syscon driver based on regmap

On Wed, Aug 22, 2012 at 11:16:33PM -0600, 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.
I think it's good idea.

Thanks
Richard

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/