Re: [PATCH V2] mpc512x/clock: fix clk_get logic

2009-11-02 Thread Grant Likely
On Fri, Oct 30, 2009 at 10:53 AM, Wolfram Sang w.s...@pengutronix.de wrote:
 +       bool id_matched = !id;
 +       bool dev_matched = !dev;
[...]
 +                       dev_matched = true;
 +               if (id  strcmp(id, p-name) == 0)
 +                       id_matched = true;

Using bool/true/false doesn't seem to be a common pattern in the
kernel.  Anyone know what the winds of prevailing opinion are
regarding 'bool' in kernel code?

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


Re: [PATCH V2] mpc512x/clock: fix clk_get logic

2009-11-02 Thread Stephen Rothwell
Hi Grant,

On Mon, 2 Nov 2009 10:48:58 -0700 Grant Likely grant.lik...@secretlab.ca 
wrote:

 Using bool/true/false doesn't seem to be a common pattern in the
 kernel.  Anyone know what the winds of prevailing opinion are
 regarding 'bool' in kernel code?

Its a good thing.

-- 
Cheers,
Stephen Rothwells...@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/


pgpD4kRkGuC1F.pgp
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH V2] mpc512x/clock: fix clk_get logic

2009-11-02 Thread Grant Likely
On Mon, Nov 2, 2009 at 4:10 PM, Stephen Rothwell s...@canb.auug.org.au wrote:
 Hi Grant,

 On Mon, 2 Nov 2009 10:48:58 -0700 Grant Likely grant.lik...@secretlab.ca 
 wrote:

 Using bool/true/false doesn't seem to be a common pattern in the
 kernel.  Anyone know what the winds of prevailing opinion are
 regarding 'bool' in kernel code?

 Its a good thing.

Thanks,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


[PATCH V2] mpc512x/clock: fix clk_get logic

2009-10-30 Thread Wolfram Sang
The current matching logic returns a clock even if only one out of two
arguments matches. This is wrong as devices may utilize more than one clock, so
only the first clock out of those is returned if the dev-match alone is
considered sufficent (noticed while working on the CAN driver). The proposed
new method will:

- return -EINVAL if both arguments are NULL
- skip the relevant check if one argument is NULL (first match wins)
- otherwise both arguments need to match

Signed-off-by: Wolfram Sang w.s...@pengutronix.de
Cc: Mark Brown broo...@sirena.org.uk
Cc: Roel Kluin roel.kl...@gmail.com
Cc: Wolfgang Denk w...@denx.de
Cc: Grant Likely grant.lik...@secretlab.ca
---

After Mark's valid comment, I'll try harder ;)

 arch/powerpc/platforms/512x/clock.c |   18 ++
 1 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/platforms/512x/clock.c 
b/arch/powerpc/platforms/512x/clock.c
index 84544d0..4168457 100644
--- a/arch/powerpc/platforms/512x/clock.c
+++ b/arch/powerpc/platforms/512x/clock.c
@@ -53,19 +53,21 @@ static DEFINE_MUTEX(clocks_mutex);
 static struct clk *mpc5121_clk_get(struct device *dev, const char *id)
 {
struct clk *p, *clk = ERR_PTR(-ENOENT);
-   int dev_match = 0;
-   int id_match = 0;
+   /* If one argument is not given, skip its match */
+   bool id_matched = !id;
+   bool dev_matched = !dev;
 
-   if (dev == NULL || id == NULL)
-   return NULL;
+   /* We need at least one argument */
+   if (!dev  !id)
+   return ERR_PTR(-EINVAL);
 
mutex_lock(clocks_mutex);
list_for_each_entry(p, clocks, node) {
if (dev == p-dev)
-   dev_match++;
-   if (strcmp(id, p-name) == 0)
-   id_match++;
-   if ((dev_match || id_match)  try_module_get(p-owner)) {
+   dev_matched = true;
+   if (id  strcmp(id, p-name) == 0)
+   id_matched = true;
+   if (dev_matched  id_matched  try_module_get(p-owner)) {
clk = p;
break;
}
-- 
1.6.3.3

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev