> -----Original Message-----
> From: Antonio Ospite [mailto:[email protected]]
> Sent: 28 May, 2015 18:58
> To: Tirdea, Irina
> Cc: Dmitry Torokhov; Bastien Nocera; [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCH 2/9] input: goodix: fix variable length array warning
>
> On Thu, 28 May 2015 15:47:38 +0300
> Irina Tirdea <[email protected]> wrote:
>
> > Fix sparse warning:
> > drivers/input/touchscreen/goodix.c:182:26: warning:
> > Variable length array is used.
> >
> > Replace the variable length array with fixed length.
> >
> > Signed-off-by: Irina Tirdea <[email protected]>
> > ---
> > drivers/input/touchscreen/goodix.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
> > index c2e785c..dac1b3c 100644
> > --- a/drivers/input/touchscreen/goodix.c
> > +++ b/drivers/input/touchscreen/goodix.c
> > @@ -147,7 +147,7 @@ static void goodix_ts_report_touch(struct goodix_ts_data *ts, u8 *coor_data)
> > */
> > static void goodix_process_events(struct goodix_ts_data *ts)
> > {
> > - u8 point_data[1 + GOODIX_CONTACT_SIZE * ts->max_touch_num];
> > + u8 point_data[1 + GOODIX_CONTACT_SIZE * GOODIX_MAX_CONTACTS];
>
> Hi,
>
Hi Antonio,
> this fixes the warning from sparse, but also changes the semantics of
> the code: ts->max_touch_num is less that GOODIX_MAX_CONTACTS for 5
> touches devices and in this case we'll end up using more memory than is
> necessary.
>
I wasn't sure if it is better to save the 5 bytes or fix the warning, so I sent this to get some more input.
Thanks for the feedback, I will drop this patch.
Thanks,
Irina
> Thanks,
> Antonio
>
> > int touch_num;
> > int i;
> >
> > --
> > 1.9.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-input" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> Antonio Ospite
> http://ao2.it
>
> A: Because it messes up the order in which people normally read text.
> See http://en.wikipedia.org/wiki/Posting_style
> Q: Why is top-posting such a bad thing?
On Wed, 3 Jun 2015 10:26:47 +0000
"Tirdea, Irina" <[email protected]> wrote:
> > -----Original Message-----
> > From: Antonio Ospite [mailto:[email protected]]
> > Sent: 28 May, 2015 18:58
> > To: Tirdea, Irina
> > Cc: Dmitry Torokhov; Bastien Nocera; [email protected]; [email protected]; [email protected]
> > Subject: Re: [PATCH 2/9] input: goodix: fix variable length array warning
> >
> > On Thu, 28 May 2015 15:47:38 +0300
> > Irina Tirdea <[email protected]> wrote:
> >
> > > Fix sparse warning:
> > > drivers/input/touchscreen/goodix.c:182:26: warning:
> > > Variable length array is used.
> > >
> > > Replace the variable length array with fixed length.
> > >
> > > Signed-off-by: Irina Tirdea <[email protected]>
> > > ---
> > > drivers/input/touchscreen/goodix.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
> > > index c2e785c..dac1b3c 100644
> > > --- a/drivers/input/touchscreen/goodix.c
> > > +++ b/drivers/input/touchscreen/goodix.c
> > > @@ -147,7 +147,7 @@ static void goodix_ts_report_touch(struct goodix_ts_data *ts, u8 *coor_data)
> > > */
> > > static void goodix_process_events(struct goodix_ts_data *ts)
> > > {
> > > - u8 point_data[1 + GOODIX_CONTACT_SIZE * ts->max_touch_num];
> > > + u8 point_data[1 + GOODIX_CONTACT_SIZE * GOODIX_MAX_CONTACTS];
> >
> > Hi,
> >
>
> Hi Antonio,
>
> > this fixes the warning from sparse, but also changes the semantics of
> > the code: ts->max_touch_num is less that GOODIX_MAX_CONTACTS for 5
> > touches devices and in this case we'll end up using more memory than is
> > necessary.
> >
>
> I wasn't sure if it is better to save the 5 bytes or fix the warning,
> so I sent this to get some more input.
> Thanks for the feedback, I will drop this patch.
>
Use kmalloc() or, alternatively, add at least a comment telling why you
think that sacrificing a few bytes —only for some devices— has
advantages over dynamic allocation.
I am not necessarily against the static allocation change, I was just
pointing out the issue.
Thanks,
Antonio
--
Antonio Ospite
http://ao2.it
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
> -----Original Message-----
> From: [email protected] [mailto:[email protected]] On Behalf Of Antonio Ospite
> Sent: 03 June, 2015 23:50
> To: Tirdea, Irina
> Cc: Dmitry Torokhov; Bastien Nocera; [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCH 2/9] input: goodix: fix variable length array warning
>
> On Wed, 3 Jun 2015 10:26:47 +0000
> "Tirdea, Irina" <[email protected]> wrote:
>
> > > -----Original Message-----
> > > From: Antonio Ospite [mailto:[email protected]]
> > > Sent: 28 May, 2015 18:58
> > > To: Tirdea, Irina
> > > Cc: Dmitry Torokhov; Bastien Nocera; [email protected]; [email protected]; [email protected]
> > > Subject: Re: [PATCH 2/9] input: goodix: fix variable length array warning
> > >
> > > On Thu, 28 May 2015 15:47:38 +0300
> > > Irina Tirdea <[email protected]> wrote:
> > >
> > > > Fix sparse warning:
> > > > drivers/input/touchscreen/goodix.c:182:26: warning:
> > > > Variable length array is used.
> > > >
> > > > Replace the variable length array with fixed length.
> > > >
> > > > Signed-off-by: Irina Tirdea <[email protected]>
> > > > ---
> > > > drivers/input/touchscreen/goodix.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
> > > > index c2e785c..dac1b3c 100644
> > > > --- a/drivers/input/touchscreen/goodix.c
> > > > +++ b/drivers/input/touchscreen/goodix.c
> > > > @@ -147,7 +147,7 @@ static void goodix_ts_report_touch(struct goodix_ts_data *ts, u8 *coor_data)
> > > > */
> > > > static void goodix_process_events(struct goodix_ts_data *ts)
> > > > {
> > > > - u8 point_data[1 + GOODIX_CONTACT_SIZE * ts->max_touch_num];
> > > > + u8 point_data[1 + GOODIX_CONTACT_SIZE * GOODIX_MAX_CONTACTS];
> > >
> > > Hi,
> > >
> >
> > Hi Antonio,
> >
> > > this fixes the warning from sparse, but also changes the semantics of
> > > the code: ts->max_touch_num is less that GOODIX_MAX_CONTACTS for 5
> > > touches devices and in this case we'll end up using more memory than is
> > > necessary.
> > >
> >
> > I wasn't sure if it is better to save the 5 bytes or fix the warning,
> > so I sent this to get some more input.
> > Thanks for the feedback, I will drop this patch.
> >
>
> Use kmalloc() or, alternatively, add at least a comment telling why you
> think that sacrificing a few bytes —only for some devices— has
> advantages over dynamic allocation.
>
You are right, kmalloc will solve both problems - the sparse warning and allocating
more bytes than necessary. Don't know why I did not think of that.
Will use that in v2.
Thanks,
Irina
> I am not necessarily against the static allocation change, I was just
> pointing out the issue.
>
> Thanks,
> Antonio
>
> --
> Antonio Ospite
> http://ao2.it
>
> A: Because it messes up the order in which people normally read text.
> See http://en.wikipedia.org/wiki/Posting_style
> Q: Why is top-posting such a bad thing?
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On Fri, Jun 05, 2015 at 04:34:38PM +0000, Tirdea, Irina wrote:
>
>
> > -----Original Message-----
> > From: [email protected] [mailto:[email protected]] On Behalf Of Antonio Ospite
> > Sent: 03 June, 2015 23:50
> > To: Tirdea, Irina
> > Cc: Dmitry Torokhov; Bastien Nocera; [email protected]; [email protected]; [email protected]
> > Subject: Re: [PATCH 2/9] input: goodix: fix variable length array warning
> >
> > On Wed, 3 Jun 2015 10:26:47 +0000
> > "Tirdea, Irina" <[email protected]> wrote:
> >
> > > > -----Original Message-----
> > > > From: Antonio Ospite [mailto:[email protected]]
> > > > Sent: 28 May, 2015 18:58
> > > > To: Tirdea, Irina
> > > > Cc: Dmitry Torokhov; Bastien Nocera; [email protected]; [email protected]; [email protected]
> > > > Subject: Re: [PATCH 2/9] input: goodix: fix variable length array warning
> > > >
> > > > On Thu, 28 May 2015 15:47:38 +0300
> > > > Irina Tirdea <[email protected]> wrote:
> > > >
> > > > > Fix sparse warning:
> > > > > drivers/input/touchscreen/goodix.c:182:26: warning:
> > > > > Variable length array is used.
> > > > >
> > > > > Replace the variable length array with fixed length.
> > > > >
> > > > > Signed-off-by: Irina Tirdea <[email protected]>
> > > > > ---
> > > > > drivers/input/touchscreen/goodix.c | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
> > > > > index c2e785c..dac1b3c 100644
> > > > > --- a/drivers/input/touchscreen/goodix.c
> > > > > +++ b/drivers/input/touchscreen/goodix.c
> > > > > @@ -147,7 +147,7 @@ static void goodix_ts_report_touch(struct goodix_ts_data *ts, u8 *coor_data)
> > > > > */
> > > > > static void goodix_process_events(struct goodix_ts_data *ts)
> > > > > {
> > > > > - u8 point_data[1 + GOODIX_CONTACT_SIZE * ts->max_touch_num];
> > > > > + u8 point_data[1 + GOODIX_CONTACT_SIZE * GOODIX_MAX_CONTACTS];
> > > >
> > > > Hi,
> > > >
> > >
> > > Hi Antonio,
> > >
> > > > this fixes the warning from sparse, but also changes the semantics of
> > > > the code: ts->max_touch_num is less that GOODIX_MAX_CONTACTS for 5
> > > > touches devices and in this case we'll end up using more memory than is
> > > > necessary.
> > > >
> > >
> > > I wasn't sure if it is better to save the 5 bytes or fix the warning,
> > > so I sent this to get some more input.
> > > Thanks for the feedback, I will drop this patch.
> > >
> >
> > Use kmalloc() or, alternatively, add at least a comment telling why you
> > think that sacrificing a few bytes —only for some devices— has
> > advantages over dynamic allocation.
> >
>
> You are right, kmalloc will solve both problems - the sparse warning and allocating
> more bytes than necessary. Don't know why I did not think of that.
> Will use that in v2.
Please leave the patch as is. We can spare 80 bytes on the stack given
that we are running in threaded IRQ. kmallocing will use more code and
runtime resources and won't give any benefits.
Thanks.
--
Dmitry
> -----Original Message-----
> From: Dmitry Torokhov [mailto:[email protected]]
> Sent: 05 June, 2015 19:41
> To: Tirdea, Irina
> Cc: 'Antonio Ospite'; Bastien Nocera; [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCH 2/9] input: goodix: fix variable length array warning
>
> On Fri, Jun 05, 2015 at 04:34:38PM +0000, Tirdea, Irina wrote:
> >
> >
> > > -----Original Message-----
> > > From: [email protected] [mailto:[email protected]] On Behalf Of Antonio Ospite
> > > Sent: 03 June, 2015 23:50
> > > To: Tirdea, Irina
> > > Cc: Dmitry Torokhov; Bastien Nocera; [email protected]; [email protected]; [email protected]
> > > Subject: Re: [PATCH 2/9] input: goodix: fix variable length array warning
> > >
> > > On Wed, 3 Jun 2015 10:26:47 +0000
> > > "Tirdea, Irina" <[email protected]> wrote:
> > >
> > > > > -----Original Message-----
> > > > > From: Antonio Ospite [mailto:[email protected]]
> > > > > Sent: 28 May, 2015 18:58
> > > > > To: Tirdea, Irina
> > > > > Cc: Dmitry Torokhov; Bastien Nocera; [email protected]; [email protected]; [email protected]
> > > > > Subject: Re: [PATCH 2/9] input: goodix: fix variable length array warning
> > > > >
> > > > > On Thu, 28 May 2015 15:47:38 +0300
> > > > > Irina Tirdea <[email protected]> wrote:
> > > > >
> > > > > > Fix sparse warning:
> > > > > > drivers/input/touchscreen/goodix.c:182:26: warning:
> > > > > > Variable length array is used.
> > > > > >
> > > > > > Replace the variable length array with fixed length.
> > > > > >
> > > > > > Signed-off-by: Irina Tirdea <[email protected]>
> > > > > > ---
> > > > > > drivers/input/touchscreen/goodix.c | 2 +-
> > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
> > > > > > index c2e785c..dac1b3c 100644
> > > > > > --- a/drivers/input/touchscreen/goodix.c
> > > > > > +++ b/drivers/input/touchscreen/goodix.c
> > > > > > @@ -147,7 +147,7 @@ static void goodix_ts_report_touch(struct goodix_ts_data *ts, u8 *coor_data)
> > > > > > */
> > > > > > static void goodix_process_events(struct goodix_ts_data *ts)
> > > > > > {
> > > > > > - u8 point_data[1 + GOODIX_CONTACT_SIZE * ts->max_touch_num];
> > > > > > + u8 point_data[1 + GOODIX_CONTACT_SIZE * GOODIX_MAX_CONTACTS];
> > > > >
> > > > > Hi,
> > > > >
> > > >
> > > > Hi Antonio,
> > > >
> > > > > this fixes the warning from sparse, but also changes the semantics of
> > > > > the code: ts->max_touch_num is less that GOODIX_MAX_CONTACTS for 5
> > > > > touches devices and in this case we'll end up using more memory than is
> > > > > necessary.
> > > > >
> > > >
> > > > I wasn't sure if it is better to save the 5 bytes or fix the warning,
> > > > so I sent this to get some more input.
> > > > Thanks for the feedback, I will drop this patch.
> > > >
> > >
> > > Use kmalloc() or, alternatively, add at least a comment telling why you
> > > think that sacrificing a few bytes —only for some devices— has
> > > advantages over dynamic allocation.
> > >
> >
> > You are right, kmalloc will solve both problems - the sparse warning and allocating
> > more bytes than necessary. Don't know why I did not think of that.
> > Will use that in v2.
>
> Please leave the patch as is. We can spare 80 bytes on the stack given
> that we are running in threaded IRQ. kmallocing will use more code and
> runtime resources and won't give any benefits.
I was actually thinking of allocating it with devm_kzalloc just once at device init and
storing a pointer to it in the goodix_ts_data structure.
Thanks,
Irina
>
> Thanks.
>
> --
> Dmitry
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On Fri, Jun 05, 2015 at 05:00:05PM +0000, Tirdea, Irina wrote:
>
>
> > -----Original Message-----
> > From: Dmitry Torokhov [mailto:[email protected]]
> > Sent: 05 June, 2015 19:41
> > To: Tirdea, Irina
> > Cc: 'Antonio Ospite'; Bastien Nocera; [email protected]; [email protected]; [email protected]
> > Subject: Re: [PATCH 2/9] input: goodix: fix variable length array warning
> >
> > On Fri, Jun 05, 2015 at 04:34:38PM +0000, Tirdea, Irina wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: [email protected] [mailto:[email protected]] On Behalf Of Antonio Ospite
> > > > Sent: 03 June, 2015 23:50
> > > > To: Tirdea, Irina
> > > > Cc: Dmitry Torokhov; Bastien Nocera; [email protected]; [email protected]; [email protected]
> > > > Subject: Re: [PATCH 2/9] input: goodix: fix variable length array warning
> > > >
> > > > On Wed, 3 Jun 2015 10:26:47 +0000
> > > > "Tirdea, Irina" <[email protected]> wrote:
> > > >
> > > > > > -----Original Message-----
> > > > > > From: Antonio Ospite [mailto:[email protected]]
> > > > > > Sent: 28 May, 2015 18:58
> > > > > > To: Tirdea, Irina
> > > > > > Cc: Dmitry Torokhov; Bastien Nocera; [email protected]; [email protected]; [email protected]
> > > > > > Subject: Re: [PATCH 2/9] input: goodix: fix variable length array warning
> > > > > >
> > > > > > On Thu, 28 May 2015 15:47:38 +0300
> > > > > > Irina Tirdea <[email protected]> wrote:
> > > > > >
> > > > > > > Fix sparse warning:
> > > > > > > drivers/input/touchscreen/goodix.c:182:26: warning:
> > > > > > > Variable length array is used.
> > > > > > >
> > > > > > > Replace the variable length array with fixed length.
> > > > > > >
> > > > > > > Signed-off-by: Irina Tirdea <[email protected]>
> > > > > > > ---
> > > > > > > drivers/input/touchscreen/goodix.c | 2 +-
> > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
> > > > > > > index c2e785c..dac1b3c 100644
> > > > > > > --- a/drivers/input/touchscreen/goodix.c
> > > > > > > +++ b/drivers/input/touchscreen/goodix.c
> > > > > > > @@ -147,7 +147,7 @@ static void goodix_ts_report_touch(struct goodix_ts_data *ts, u8 *coor_data)
> > > > > > > */
> > > > > > > static void goodix_process_events(struct goodix_ts_data *ts)
> > > > > > > {
> > > > > > > - u8 point_data[1 + GOODIX_CONTACT_SIZE * ts->max_touch_num];
> > > > > > > + u8 point_data[1 + GOODIX_CONTACT_SIZE * GOODIX_MAX_CONTACTS];
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > >
> > > > > Hi Antonio,
> > > > >
> > > > > > this fixes the warning from sparse, but also changes the semantics of
> > > > > > the code: ts->max_touch_num is less that GOODIX_MAX_CONTACTS for 5
> > > > > > touches devices and in this case we'll end up using more memory than is
> > > > > > necessary.
> > > > > >
> > > > >
> > > > > I wasn't sure if it is better to save the 5 bytes or fix the warning,
> > > > > so I sent this to get some more input.
> > > > > Thanks for the feedback, I will drop this patch.
> > > > >
> > > >
> > > > Use kmalloc() or, alternatively, add at least a comment telling why you
> > > > think that sacrificing a few bytes —only for some devices— has
> > > > advantages over dynamic allocation.
> > > >
> > >
> > > You are right, kmalloc will solve both problems - the sparse warning and allocating
> > > more bytes than necessary. Don't know why I did not think of that.
> > > Will use that in v2.
> >
> > Please leave the patch as is. We can spare 80 bytes on the stack given
> > that we are running in threaded IRQ. kmallocing will use more code and
> > runtime resources and won't give any benefits.
>
> I was actually thinking of allocating it with devm_kzalloc just once at device init and
> storing a pointer to it in the goodix_ts_data structure.
Still, why? Even if you do this once you will need both code and runtime
resources to bookkeeping whereas allocating 80 bytes on stack are just a
matter of register addition. And because we are running in threaded IRQ
context there is no concern of blowing up the limited stack.
The story would be different if you needed, let's say 800 or 1600 bytes.
Thanks.
--
Dmitry
> -----Original Message-----
> From: Dmitry Torokhov [mailto:[email protected]]
> Sent: 05 June, 2015 20:12
> To: Tirdea, Irina
> Cc: 'Antonio Ospite'; Bastien Nocera; [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCH 2/9] input: goodix: fix variable length array warning
>
> On Fri, Jun 05, 2015 at 05:00:05PM +0000, Tirdea, Irina wrote:
> >
> >
> > > -----Original Message-----
> > > From: Dmitry Torokhov [mailto:[email protected]]
> > > Sent: 05 June, 2015 19:41
> > > To: Tirdea, Irina
> > > Cc: 'Antonio Ospite'; Bastien Nocera; [email protected]; [email protected]; [email protected]
> > > Subject: Re: [PATCH 2/9] input: goodix: fix variable length array warning
> > >
> > > On Fri, Jun 05, 2015 at 04:34:38PM +0000, Tirdea, Irina wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: [email protected] [mailto:[email protected]] On Behalf Of Antonio Ospite
> > > > > Sent: 03 June, 2015 23:50
> > > > > To: Tirdea, Irina
> > > > > Cc: Dmitry Torokhov; Bastien Nocera; [email protected]; [email protected]; [email protected]
> > > > > Subject: Re: [PATCH 2/9] input: goodix: fix variable length array warning
> > > > >
> > > > > On Wed, 3 Jun 2015 10:26:47 +0000
> > > > > "Tirdea, Irina" <[email protected]> wrote:
> > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Antonio Ospite [mailto:[email protected]]
> > > > > > > Sent: 28 May, 2015 18:58
> > > > > > > To: Tirdea, Irina
> > > > > > > Cc: Dmitry Torokhov; Bastien Nocera; [email protected]; [email protected]; linux-
> [email protected]
> > > > > > > Subject: Re: [PATCH 2/9] input: goodix: fix variable length array warning
> > > > > > >
> > > > > > > On Thu, 28 May 2015 15:47:38 +0300
> > > > > > > Irina Tirdea <[email protected]> wrote:
> > > > > > >
> > > > > > > > Fix sparse warning:
> > > > > > > > drivers/input/touchscreen/goodix.c:182:26: warning:
> > > > > > > > Variable length array is used.
> > > > > > > >
> > > > > > > > Replace the variable length array with fixed length.
> > > > > > > >
> > > > > > > > Signed-off-by: Irina Tirdea <[email protected]>
> > > > > > > > ---
> > > > > > > > drivers/input/touchscreen/goodix.c | 2 +-
> > > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > > >
> > > > > > > > diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
> > > > > > > > index c2e785c..dac1b3c 100644
> > > > > > > > --- a/drivers/input/touchscreen/goodix.c
> > > > > > > > +++ b/drivers/input/touchscreen/goodix.c
> > > > > > > > @@ -147,7 +147,7 @@ static void goodix_ts_report_touch(struct goodix_ts_data *ts, u8 *coor_data)
> > > > > > > > */
> > > > > > > > static void goodix_process_events(struct goodix_ts_data *ts)
> > > > > > > > {
> > > > > > > > - u8 point_data[1 + GOODIX_CONTACT_SIZE * ts->max_touch_num];
> > > > > > > > + u8 point_data[1 + GOODIX_CONTACT_SIZE * GOODIX_MAX_CONTACTS];
> > > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > >
> > > > > > Hi Antonio,
> > > > > >
> > > > > > > this fixes the warning from sparse, but also changes the semantics of
> > > > > > > the code: ts->max_touch_num is less that GOODIX_MAX_CONTACTS for 5
> > > > > > > touches devices and in this case we'll end up using more memory than is
> > > > > > > necessary.
> > > > > > >
> > > > > >
> > > > > > I wasn't sure if it is better to save the 5 bytes or fix the warning,
> > > > > > so I sent this to get some more input.
> > > > > > Thanks for the feedback, I will drop this patch.
> > > > > >
> > > > >
> > > > > Use kmalloc() or, alternatively, add at least a comment telling why you
> > > > > think that sacrificing a few bytes —only for some devices— has
> > > > > advantages over dynamic allocation.
> > > > >
> > > >
> > > > You are right, kmalloc will solve both problems - the sparse warning and allocating
> > > > more bytes than necessary. Don't know why I did not think of that.
> > > > Will use that in v2.
> > >
> > > Please leave the patch as is. We can spare 80 bytes on the stack given
> > > that we are running in threaded IRQ. kmallocing will use more code and
> > > runtime resources and won't give any benefits.
> >
> > I was actually thinking of allocating it with devm_kzalloc just once at device init and
> > storing a pointer to it in the goodix_ts_data structure.
>
> Still, why? Even if you do this once you will need both code and runtime
> resources to bookkeeping whereas allocating 80 bytes on stack are just a
> matter of register addition. And because we are running in threaded IRQ
> context there is no concern of blowing up the limited stack.
>
> The story would be different if you needed, let's say 800 or 1600 bytes.
>
Ok, I see your point. Will leave the patch as is.
Thanks,
Irina
> Thanks.
>
> --
> Dmitry
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?