On 08/20/2012 01:06 PM, AnilKumar, Chimata wrote: > On Mon, Aug 20, 2012 at 16:33:43, AnilKumar, Chimata wrote: >> On Mon, Aug 20, 2012 at 16:27:53, Marc Kleine-Budde wrote: >>> On 08/20/2012 12:41 PM, Marc Kleine-Budde wrote: >>>> On 08/20/2012 10:45 AM, AnilKumar Ch wrote: >>>>> Add Runtime PM support to C_CAN/D_CAN controller. The runtime PM >>>>> APIs control clocks for C_CAN/D_CAN IP and prevent access to the >>>>> register of C_CAN/D_CAN IP when clock is turned off. >>>>> >>>>> Signed-off-by: AnilKumar Ch <anilkumar@xxxxxx> >>>>> --- >>>>> This patch has been tested on AM335X EVM. Due to lack of hardware >>>>> I am not able to test c_can functionality. I appreciate if anyone >>>>> can test c_can functionality with this patch. >>>>> >>>>> This patch is based on "can-next/master" >>>>> >>>>> Changes from v7: >>>>> - Incorporated Marc's commets on v7 >>>>> * changed device pointer to c_can_priv pointer >>>> >>>> looks good. >>>> >>>> [...] >>>> >>>>> --- a/drivers/net/can/c_can/c_can_platform.c >>>>> +++ b/drivers/net/can/c_can/c_can_platform.c >>>>> @@ -32,6 +32,7 @@ >>>>> #include <linux/clk.h> >>>>> #include <linux/of.h> >>>>> #include <linux/of_device.h> >>>>> +#include <linux/pm_runtime.h> >>>>> >>>>> #include <linux/can/dev.h> >>>>> >>>>> @@ -177,8 +178,11 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev) >>>>> goto exit_free_device; >>>> ^^^^^^^^^^^^^^^^ >>>>> } >>>>> >>>>> + pm_runtime_enable(&pdev->dev); >>>>> + >>>>> dev->irq = irq; >>>>> priv->base = addr; >>>>> + priv->device = &pdev->dev; >>>>> priv->can.clock.freq = clk_get_rate(clk); >>>>> priv->priv = clk; >>>>> >>>>> @@ -198,6 +202,7 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev) >>>>> >>>>> exit_free_device: >>>>> platform_set_drvdata(pdev, NULL); >>>>> + pm_runtime_disable(&pdev->dev); >>>> >>>> I think this will end up in a NULL pointer deferf. >>> >>> Should be no problem, as you're using pdev->dev not priv->device. >>> >>> Sorry for the noise :) >>> >> >> One more catch here, return path is not proper. >> >> exit_free_device: should contain only free_c_can_dev(dev); > > And second goto exit_free_device; call should modify to > goto exit_clear_drvdata; > > return path looks like this > > exit_clear_drvdata: > platform_set_drvdata(pdev, NULL); > pm_runtime_disable(&pdev->dev); > exit_free_device: > free_c_can_dev(dev); > ... yes, please send a V9 -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
Attachment:
signature.asc
Description: OpenPGP digital signature
