On Fri, Jan 7, 2011 at 10:38 PM, Jamie Iles <jamie@xxxxxxxxxxxxx> wrote:
>> > +static int dw_wdt_open(struct inode *inode, struct file *filp)
>> > +{
>> > + Â Â Â /* Make sure we don't get unloaded. */
>> > + Â Â Â __module_get(THIS_MODULE);
>> > +
>> > + Â Â Â spin_lock(&dw_wdt.lock);
>> > + Â Â Â if (!dw_wdt_is_enabled()) {
>> > + Â Â Â Â Â Â Â /*
>> > + Â Â Â Â Â Â Â Â* The watchdog is not currently enabled. Set the timeout to
>> > + Â Â Â Â Â Â Â Â* the maximum and then start it.
>> > + Â Â Â Â Â Â Â Â*/
>> > + Â Â Â Â Â Â Â dw_wdt_set_top(DW_WDT_MAX_TOP);
>>
>> shouldn't we check return value here??
>
> No, dw_wdt_set_top() can't fail, it just returns the timeout period that
> it set in seconds. ÂWe use this when the user changes the timeout period
> as we may not be able to select the exact timeout period they chose, but
> here we just select the maximum timeout.
Sorry, I missed that.
>> > +static int __devinit dw_wdt_drv_probe(struct platform_device *pdev)
>> > +{
>> > + Â Â Â int ret;
>> > + Â Â Â struct resource *mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> > +
>> > + Â Â Â if (!mem)
>> > + Â Â Â Â Â Â Â return -EINVAL;
>> > +
>> > + Â Â Â if (!devm_request_mem_region(&pdev->dev, mem->start, resource_size(mem),
>> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â"iomem"))
>> > + Â Â Â Â Â Â Â return -ENOMEM;
>> > +
>> > + Â Â Â dw_wdt.regs = devm_ioremap(&pdev->dev, mem->start,
>> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âresource_size(mem));
>> > + Â Â Â if (!dw_wdt.regs)
>> > + Â Â Â Â Â Â Â return -ENOMEM;
>>
>> should release mem_region in case of error.
>>
>> > +
>> > + Â Â Â dw_wdt.clk = clk_get(&pdev->dev, NULL);
>> > + Â Â Â if (IS_ERR_OR_NULL(dw_wdt.clk))
>> > + Â Â Â Â Â Â Â return -ENODEV;
>>
>> should release mem_region and free ioremaped space.
>
> We're using devres for the ioremap and region request so that takes care
> of the cleanup for us. ÂThis also means we don't need to iounmap and
> release in the release method.
ok
>
>> Also, may be we can continue here in case of error too.
>> Some platforms might nor support clock framework. You can enable and
>> disable clk's if dw_wdt.clk is !NULL
>
> The DesignWare watchdog has 16 timeout periods and these are derived
> from the clock frequency input to the WDT. ÂIf we don't have a clk then
> we don't know how long the timeout periods. ÂThe only alternative would
> be to have a 'struct dw_wdt_platform_data' that includes the input clock
> frequency which we use if we can't get a clk and get the rate.
That will be fine.
--
viresh
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
