lkml.org 
[lkml]   [2016]   [Feb]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2] irqchip: irq-mvebu-odmi: new driver for platform MSI on Marvell 7K/8K
Marc,

On Thu, 18 Feb 2016 16:41:23 +0000, Marc Zyngier wrote:

> It looks really nice, except for a couple of points, see below.

Thanks again for the review.

> > +/*
> > + * We don't support the group events, so we simply have 8 interrupts
> > + * per frame.
> > + */
> > +#define NODMIS_PER_FRAME 8
> > +#define NODMIS_SHIFT 3
> > +#define NODMIS_MASK 7
>
> All these values are directly related, so it would be nice if they would
> be expressed in term of each other:
>
> #define NODMIS_SHIFT 3
> #define NODMIS_PER_FRAME (1 << NODMIS_SHIFT)
> #define NODMIS_MASK (NODMIS_PER_FRAME - 1)

Good point, will do.

> > + odmis_bm = kzalloc(odmis_count * NODMIS_PER_FRAME / BITS_PER_BYTE,
> > + GFP_KERNEL);
>
> Blah. this will allocate the exact number of bytes, which you will then
> access as longs, touching memory that's not yours in the process (I've
> been recently bitten and publicly shamed...).

Aah, yes.

> Consider the following:
>
> odmis_bm = kzalloc(BITS_TO_LONGS(odmis_count * NODMIS_PER_FRAME) *
> sizeof(long), GFP_KERNEL);

Will do.

Thanks!

Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

\
 
 \ /
  Last update: 2016-02-19 11:01    [W:0.169 / U:0.252 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site