linux-kernel - Re: [PATCH V2 4/9] ata/sata_mv: Remove conditional compilation of clk code

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120424072653.GB24089@lunn.ch>
Date:	Tue, 24 Apr 2012 09:26:53 +0200
From:	Andrew Lunn <andrew@...n.ch>
To:	Viresh Kumar <viresh.kumar@...com>
Cc:	Andrew Lunn <andrew@...n.ch>,
	"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
	"linux@....linux.org.uk" <linux@....linux.org.uk>,
	"sshtylyov@...sta.com" <sshtylyov@...sta.com>,
	spear-devel <spear-devel@...t.st.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-ide@...r.kernel.org" <linux-ide@...r.kernel.org>,
	"viresh.linux@...il.com" <viresh.linux@...il.com>,
	"mturquette@...aro.org" <mturquette@...aro.org>,
	"jgarzik@...hat.com" <jgarzik@...hat.com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH V2 4/9] ata/sata_mv: Remove conditional compilation of
 clk code

On Tue, Apr 24, 2012 at 12:35:23PM +0530, Viresh Kumar wrote:
> On 4/24/2012 12:34 PM, Andrew Lunn wrote:
> > I don't think this change is correct. With the old semantics, it was:
> 
> Sorry. :(
> 
> > If we have CLK support, we expect there to be a clock for sata_mv, and
> > if there is no such clock, output a notice message, something is
> > probably wrong, i expected there to be a clock.
> > 
> > The new semantics are:
> > 
> > We expect there to be a clock for sata_mv, and if there is no such
> > clock, output a notice message, something is probably wrong, i
> > expected there to be a clock.
> > 
> > We are going to see this notice message much more, when it is not
> > expected.
> 
> So, the only problem is this message?
> How do you suggest to tackle this now. Have #ifdef,#endif around this print?

Well, adding #ifdef defeats the point of adding dummy implementations.

Maybe, rather than return -ENODEV, return -ENOTSUP.

IS_ERR() still returns true, so in most cases, no code needs
changing. However, when you need to differentiate between, "clock does
not exists" and "Dummy clock functions being used", you can tell the
difference. mv_sata could look like:

        hpriv->clk = clk_get(&pdev->dev, NULL);
        if (IS_ERR(hpriv->clk))
	        if (PTR_ERR(hpriv->clk) == -ENODEV)
		        dev_notice(&pdev->dev, "cannot get clkdev\n");
        else
                clk_prepare_enable(hpriv->clk);


You would however, need to look at all uses of clk_get and see if any
are looked for ENODEV, and not just IS_ERR(), and fix those....

	Andrew

--
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/