On Thu, 4 Dec 2008, Darius wrote:
> > > > + if (!request_mem_region(res->start, res_size, res->name)) {
> > > > + dev_err(&pdev->dev, "can't allocate %d bytes at %d
> > > > address\n",
> > > > + res_size, res->start);
> > > > + return -ENOMEM;
> > > > + }
> > > I was once told, one doesn't need request_mem_region() on regions from
> > > platform data resources - this is already done implicitly.
> >
> > I belive that request_mem_region() is the correct thing to do, IIRC,
> > the platform_device_register() calls insert_resource() on all the
> > resource regions of the device so that it is guarnateed to appear in
> > the /proc/ioport or /proc/iomem map. However I belive this does not
> > actually make a reservation of that resource.
> >
>
> confused... Guennadi, can you please explain more your mention?
Look at this:
http://marc.info/?l=linux-sh&m=121642007016402&w=2
> > > > + /* Get I2C clock */
> > > > + i2c_imx->clk = clk_get(NULL, "i2c_clk");
> > > There can be several i2c busses on the system, so you want:
> > >
> > > + i2c_imx->clk = clk_get(&pdev->dev, "i2c_clk");
> >
> > tbh, the bus clock should be defined as NULL, any extra clocks for
> > a device get names. However this may need fixing in the platform
> > as well. I belive RMK is already making these changes for AMBA and
> > I will probably follow suit with the s3c architectures as soon as
> > the rest of the development work is out of the way.
Ben, what do you mean here? I thought Russell was against
platform-specific clock names and for function names. But i2c_clk is a
function name. I don't understand the difference between "bus clock" and
"extra clocks" - on i.MX31 we have 3 i2c busses with separate clock
sources, so, we have to use the platform device in clk_get to request a
specific i2c clock matching the platform-device index. That's all what I
meant.
> > > > +struct imxi2c_platform_data {
> > > > + int (*init)(struct device *dev);
> > > > + int (*exit)(struct device *dev);
> > > What are you going to use .exit() for? Is it really needed? Even if it is,
> > > it can easily return void I guess?
> > >
>
> Where to put commnets? Right here in driver code or is better in
> /Documentation?
Something like this:
+/**
+ * @init: initialise the interface
+ * @exit: free interface resources
+ */
+struct imxi2c_platform_data {
+ int (*init)(struct device *dev);
+ int (*exit)(struct device *dev);
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
