Hi Felipe,
On Sat, Feb 09, 2013 at 06:53:35PM +0200, Felipe Balbi wrote:
> On Sat, Feb 09, 2013 at 01:38:12PM -0300, Ezequiel Garcia wrote:
> > Fix gpmc_cs_reserved() so it returns 0 if the chip select
> > is available (not reserved) or an error otherwise.
> > This allows to use the return value directly and return a proper error code.
> >
> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@xxxxxxxxxxxxxxxxxx>
> > ---
> > arch/arm/mach-omap2/gpmc.c | 12 ++++++++----
> > 1 files changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
> > index bd3bc93..604c1eb 100644
> > --- a/arch/arm/mach-omap2/gpmc.c
> > +++ b/arch/arm/mach-omap2/gpmc.c
> > @@ -452,12 +452,16 @@ static int gpmc_cs_set_reserved(int cs, int reserved)
> > return 0;
> > }
> >
> > +/* Returns 0 if CS is available (not reseverd) or an error otherwise */
>
> s/reseverd/reserved/
>
Indeed.
> > static int gpmc_cs_reserved(int cs)
> > {
> > if (cs > GPMC_CS_NUM)
> > return -ENODEV;
> >
> > - return gpmc_cs_map & (1 << cs);
> > + if (gpmc_cs_map & (1 << cs))
> > + return -EBUSY;
> > +
> > + return 0;
>
> you could use a ternary operator here:
>
> bit = gpmc_cs_map & (1 << cs);
>
> return bit ? -EBUSY : 0;
>
> But to be frank, I'm not sure this makes that much sense, I'd expect
> gpmc_cs_reserved() to return 0 or 1 depending on the state (just as it
> was before). The name of the method asks for a boolean return value.
>
Well, we can change the return value to a boolean.
However, note that the function 'as it was before' was somewhat inconsistent:
gpmc_cs_reserved returned -ENODEV if the given 'cs' was out of range and
a non-negative integer otherwise, 0 if the cs is available and positive
integer otherwise.
So, what I'm doing here is fixing this by returning an appropriate error
condition or 0 if the CS is available.
If we change it to return a boolean, we won't be able to distinguish
between EBUSY and ENODEV.
Let me know what you prefer and I'll fix it on v2.
Thanks for the review,
--
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
