Messages in this thread Patch in this message | | | Date | Wed, 19 Nov 2008 00:19:32 -0800 | From | Andrew Morton <> | Subject | Re: [PATCH] Add 1-wire master driver for i.MX27 / i.MX31 |
| |
On Wed, 19 Nov 2008 01:16:13 +0300 Evgeniy Polyakov <zbr@ioremap.net> wrote:
> From: Sascha Hauer <s.hauer@pengutronix.de> > > This patch adds support for the 1-wire master interface for i.MX27 and > i.MX31.
Looks sane.
> Ack for this one, the other patches from this series need to go via > Russell King to prevent merge conflicts with other MX2 patches. Sorry, I > should have made that clear.
Is this patch fully independent of the others?
> > ... > > +static int __devinit mxc_w1_probe(struct platform_device *pdev) > +{ > + struct mxc_w1_device *mdev; > + struct resource *res; > + int err = 0; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) > + return -ENODEV; > + > + mdev = kzalloc(sizeof(struct mxc_w1_device) + > + sizeof(struct w1_bus_master), GFP_KERNEL);
whee, that's a bit of a hack. It would be better to do
struct foo { struct mxc_w1_device mxc_w1_device; struct w1_bus_master w1_bus_master; };
no?
> + if (!mdev) > + return -ENOMEM; > + > + mdev->clk = clk_get(&pdev->dev, "owire_clk"); > + if (!mdev->clk) { > + err = -ENODEV; > + goto failed_clk; > + } > + > + mdev->bus_master = (struct w1_bus_master *)(mdev + 1); > + mdev->clkdiv = (clk_get_rate(mdev->clk) / 1000000) - 1;
If that 1000000 refers to microseconds then the use of USEC_PER_SEC would be clearer.
> + res = request_mem_region(res->start, resource_size(res), > + "mxc_w1"); > + if (!res) { > + err = -EBUSY; > + goto failed_req; > + } > + > + mdev->regs = ioremap(res->start, resource_size(res)); > + if (!mdev->regs) { > + printk(KERN_ERR"Cannot map frame buffer registers\n"); > + goto failed_ioremap; > + } > + > + clk_enable(mdev->clk); > + __raw_writeb(mdev->clkdiv, mdev->regs + MXC_W1_TIME_DIVIDER); > + > + mdev->bus_master->data = mdev; > + mdev->bus_master->reset_bus = &mxc_w1_ds2_reset_bus; > + mdev->bus_master->touch_bit = &mxc_w1_ds2_touch_bit; > + > + err = w1_add_master_device(mdev->bus_master); > + > + if (err) > + goto failed_add; > + > + platform_set_drvdata(pdev, mdev); > + return 0; > + > +failed_add: > + iounmap(mdev->regs); > +failed_ioremap: > + release_mem_region(res->start, resource_size(res)); > +failed_req: > + clk_put(mdev->clk); > +failed_clk: > + kfree(mdev); > + return err; > +}
Trivial fixes:
From: Andrew Morton <akpm@linux-foundation.org>
- coding-style fixes
- remove unneeded casts of void*
Cc: Evgeniy Polyakov <zbr@ioremap.net> Cc: Luotao Fu <l.fu@pengutronix.de> Cc: Russell King <rmk@arm.linux.org.uk> Cc: Sascha Hauer <s.hauer@pengutronix.de> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> ---
drivers/w1/masters/mxc_w1.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
diff -puN drivers/w1/masters/Kconfig~add-1-wire-master-driver-for-imx27-imx31-fix drivers/w1/masters/Kconfig diff -puN drivers/w1/masters/Makefile~add-1-wire-master-driver-for-imx27-imx31-fix drivers/w1/masters/Makefile diff -puN drivers/w1/masters/mxc_w1.c~add-1-wire-master-driver-for-imx27-imx31-fix drivers/w1/masters/mxc_w1.c --- a/drivers/w1/masters/mxc_w1.c~add-1-wire-master-driver-for-imx27-imx31-fix +++ a/drivers/w1/masters/mxc_w1.c @@ -59,8 +59,7 @@ static u8 mxc_w1_ds2_reset_bus(void *dat { u8 reg_val, rc = 1; unsigned int timeout_cnt = 0; - - struct mxc_w1_device *dev = (struct mxc_w1_device *)data; + struct mxc_w1_device *dev = data; __raw_writeb(0x80, (dev->regs + MXC_W1_CONTROL)); @@ -87,8 +86,7 @@ static u8 mxc_w1_ds2_reset_bus(void *dat static u8 mxc_w1_ds2_touch_bit(void *data, u8 bit) { unsigned int timeout_cnt = 400; - - struct mxc_w1_device *mdev = (struct mxc_w1_device *)data; + struct mxc_w1_device *mdev = data; void __iomem *ctrl_addr = mdev->regs + MXC_W1_CONTROL; __raw_writeb((1 << (5 - bit)), ctrl_addr); @@ -135,9 +133,9 @@ static int __devinit mxc_w1_probe(struct goto failed_req; } - mdev->regs = ioremap(res->start, resource_size(res)); + mdev->regs = ioremap(res->start, resource_size(res)); if (!mdev->regs) { - printk(KERN_ERR"Cannot map frame buffer registers\n"); + printk(KERN_ERR "Cannot map frame buffer registers\n"); goto failed_ioremap; } _
| |