Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751838Ab1CNKPi (ORCPT ); Mon, 14 Mar 2011 06:15:38 -0400 Received: from caramon.arm.linux.org.uk ([78.32.30.218]:54247 "EHLO caramon.arm.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750993Ab1CNKPg (ORCPT ); Mon, 14 Mar 2011 06:15:36 -0400 Date: Mon, 14 Mar 2011 10:15:13 +0000 From: Russell King - ARM Linux To: Jean-Christophe PLAGNIOL-VILLARD Cc: linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org, Nicolas Ferre , linux-kernel@vger.kernel.org, Jamie Iles , Hans-Christian Egtvedt Subject: Re: [PATCH] macb: detect IP version to determin if we are on at91 or avr32 Message-ID: <20110314101512.GA26085@n2100.arm.linux.org.uk> References: <1299863585-17263-1-git-send-email-plagnioj@jcrosoft.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1299863585-17263-1-git-send-email-plagnioj@jcrosoft.com> User-Agent: Mutt/1.5.19 (2009-01-05) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2023 Lines: 58 On Fri, Mar 11, 2011 at 06:13:05PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote: > + if (macb_is_at91(bp)) { > + bp->pclk = clk_get(&pdev->dev, "macb_clk"); > + if (IS_ERR(bp->pclk)) { > + dev_err(&pdev->dev, "failed to get macb_clk\n"); > + goto err_out_free_dev; > + } > + clk_enable(bp->pclk); > + } else { > + bp->pclk = clk_get(&pdev->dev, "pclk"); > + if (IS_ERR(bp->pclk)) { > + dev_err(&pdev->dev, "failed to get pclk\n"); > + goto err_out_free_dev; > + } > + bp->hclk = clk_get(&pdev->dev, "hclk"); > + if (IS_ERR(bp->hclk)) { > + dev_err(&pdev->dev, "failed to get hclk\n"); > + goto err_out_put_pclk; > + } > + > + clk_enable(bp->pclk); > + clk_enable(bp->hclk); > + } This is the same kind of sillyness that started getting OMAP into problems with the clk API. Just do this instead: bp->pclk = clk_get(&pdev->dev, "pclk"); if (IS_ERR(bp->pclk)) { dev_err(&pdev->dev, "failed to get pclk\n"); goto err_out_free_dev; } bp->hclk = clk_get(&pdev->dev, "hclk"); if (IS_ERR(bp->hclk)) { dev_err(&pdev->dev, "failed to get hclk\n"); goto err_out_put_pclk; } clk_enable(bp->pclk); clk_enable(bp->hclk); And then require _all_ platforms using this driver to provide a pclk and a hclk for this device, whether they exist in the SoC or not. Where they don't, provide dummy clocks for it. This probably means you end up with _less_ bloat overall because you're not having to build the above code. You've less lines of source code to maintain. You have a simplified dirver with consistent requirements across all platforms. You don't need to read the version register, and you don't need macb_is_at91() and macb_is_avr32(). With clkdev it's _cheap_ to provide these dummy clocks once you have one dummy clock already in place. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/