linux-kernel - Re: [PATCH] macb: detect IP version to determin if we are on at91 or avr32

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Mon, 14 Mar 2011 18:24:14 +0100
From:	Jean-Christophe PLAGNIOL-VILLARD <plagnioj@...osoft.com>
To:	Russell King - ARM Linux <linux@....linux.org.uk>
Cc:	linux-arm-kernel@...ts.infradead.org, netdev@...r.kernel.org,
	Nicolas Ferre <nicolas.ferre@...el.com>,
	linux-kernel@...r.kernel.org, Jamie Iles <jamie@...ieiles.com>,
	Hans-Christian Egtvedt <hans-christian.egtvedt@...el.com>
Subject: Re: [PATCH] macb: detect IP version to determin if we are on at91
 or avr32

On 10:15 Mon 14 Mar     , Russell King - ARM Linux wrote:
> 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().
no we do need it for some of the register IP implementation related to the MII
at least
> 
> With clkdev it's _cheap_ to provide these dummy clocks once you have one
> dummy clock already in place.
I known and already agree about the clock, Jamie Patches will take care about it

this patch remove the ifdef ARCH and now detect the IP revision to determin
the IP specific implementation

Best Regards,
J.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/