On 08/06/2013 10:43 PM, Gerhard Sittig wrote:
> the .get_clock() callback is run from probe() and might allocate
> resources, introduce a .put_clock() callback that is run from remove()
> to undo any allocation activities
>
> prepare and enable the clocks in open(), disable and unprepare the
> clocks in close() if clocks were acquired during probe(), to not assume
> knowledge about which activities are done in probe() and remove()
>
> use devm_get_clk() to lookup the SYS and REF clocks, to have the clocks
> put upon device shutdown
>
> store pointers to data structures upon successful allocation already
> instead of deferral until complete setup, such that subroutines in the
> setup sequence may access those data structures as well to track their
> resource acquisition
>
> since clock allocation remains optional, the release callback as well as
> the enable/disable calls in open/close are optional as well
>
> Signed-off-by: Gerhard Sittig <gsi@xxxxxxx>
> ---
> drivers/net/can/mscan/mpc5xxx_can.c | 18 ++++++++++++------
> drivers/net/can/mscan/mscan.c | 27 ++++++++++++++++++++++++++-
> drivers/net/can/mscan/mscan.h | 3 +++
> 3 files changed, 41 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/can/mscan/mpc5xxx_can.c b/drivers/net/can/mscan/mpc5xxx_can.c
> index bc422ba..e59b3a3 100644
> --- a/drivers/net/can/mscan/mpc5xxx_can.c
> +++ b/drivers/net/can/mscan/mpc5xxx_can.c
> @@ -40,6 +40,7 @@ struct mpc5xxx_can_data {
> unsigned int type;
> u32 (*get_clock)(struct platform_device *ofdev, const char *clock_name,
> int *mscan_clksrc);
> + void (*put_clock)(struct platform_device *ofdev);
> };
>
> #ifdef CONFIG_PPC_MPC52xx
> @@ -180,7 +181,7 @@ static u32 mpc512x_can_get_clock(struct platform_device *ofdev,
> clockdiv = 1;
>
> if (!clock_name || !strcmp(clock_name, "sys")) {
> - sys_clk = clk_get(&ofdev->dev, "sys_clk");
> + sys_clk = devm_clk_get(&ofdev->dev, "sys_clk");
> if (IS_ERR(sys_clk)) {
> dev_err(&ofdev->dev, "couldn't get sys_clk\n");
> goto exit_unmap;
> @@ -203,7 +204,7 @@ static u32 mpc512x_can_get_clock(struct platform_device *ofdev,
> }
>
> if (clocksrc < 0) {
> - ref_clk = clk_get(&ofdev->dev, "ref_clk");
> + ref_clk = devm_clk_get(&ofdev->dev, "ref_clk");
> if (IS_ERR(ref_clk)) {
> dev_err(&ofdev->dev, "couldn't get ref_clk\n");
> goto exit_unmap;
> @@ -280,6 +281,8 @@ static int mpc5xxx_can_probe(struct platform_device *ofdev)
> dev = alloc_mscandev();
> if (!dev)
> goto exit_dispose_irq;
> + platform_set_drvdata(ofdev, dev);
> + SET_NETDEV_DEV(dev, &ofdev->dev);
>
> priv = netdev_priv(dev);
> priv->reg_base = base;
> @@ -296,8 +299,6 @@ static int mpc5xxx_can_probe(struct platform_device *ofdev)
> goto exit_free_mscan;
> }
>
> - SET_NETDEV_DEV(dev, &ofdev->dev);
> -
> err = register_mscandev(dev, mscan_clksrc);
> if (err) {
> dev_err(&ofdev->dev, "registering %s failed (err=%d)\n",
> @@ -305,8 +306,6 @@ static int mpc5xxx_can_probe(struct platform_device *ofdev)
> goto exit_free_mscan;
> }
>
> - platform_set_drvdata(ofdev, dev);
> -
> dev_info(&ofdev->dev, "MSCAN at 0x%p, irq %d, clock %d Hz\n",
> priv->reg_base, dev->irq, priv->can.clock.freq);
>
> @@ -324,10 +323,17 @@ exit_unmap_mem:
>
> static int mpc5xxx_can_remove(struct platform_device *ofdev)
> {
> + const struct of_device_id *match;
> + const struct mpc5xxx_can_data *data;
> struct net_device *dev = platform_get_drvdata(ofdev);
> struct mscan_priv *priv = netdev_priv(dev);
>
> + match = of_match_device(mpc5xxx_can_table, &ofdev->dev);
> + data = match ? match->data : NULL;
> +
> unregister_mscandev(dev);
> + if (data && data->put_clock)
> + data->put_clock(ofdev);
> iounmap(priv->reg_base);
> irq_dispose_mapping(dev->irq);
> free_candev(dev);
> diff --git a/drivers/net/can/mscan/mscan.c b/drivers/net/can/mscan/mscan.c
> index e6b4095..4f998f5 100644
> --- a/drivers/net/can/mscan/mscan.c
> +++ b/drivers/net/can/mscan/mscan.c
> @@ -573,10 +573,24 @@ static int mscan_open(struct net_device *dev)
> struct mscan_priv *priv = netdev_priv(dev);
> struct mscan_regs __iomem *regs = priv->reg_base;
>
> + if (priv->clk_ipg) {
> + ret = clk_prepare_enable(priv->clk_ipg);
> + if (ret)
> + goto exit_retcode;
> + }
> + if (priv->clk_can) {
> + ret = clk_prepare_enable(priv->clk_can);
> + if (ret) {
> + if (priv->clk_ipg)
> + clk_disable_unprepare(priv->clk_ipg);
> + goto exit_retcode;
Why don't you add another jump label and jump to that to disable the
ipkg clock?
> + }
> + }
> +
> /* common open */
> ret = open_candev(dev);
> if (ret)
> - return ret;
> + goto exit_dis_clock;
>
> napi_enable(&priv->napi);
>
> @@ -604,6 +618,12 @@ exit_free_irq:
> exit_napi_disable:
> napi_disable(&priv->napi);
> close_candev(dev);
> +exit_dis_clock:
> + if (priv->clk_can)
> + clk_disable_unprepare(priv->clk_can);
> + if (priv->clk_ipg)
> + clk_disable_unprepare(priv->clk_ipg);
> +exit_retcode:
> return ret;
> }
Marc
--
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
