Re: [PATCH 1/2] input: cyapa: fix sparse warning issue of incorrect type in assiggment
Dudley,
On Thu, Jan 22, 2015 at 09:57:42AM +, Dudley Du wrote:
> > -Original Message-
> > From: Jeremiah Mahler [mailto:jmmah...@gmail.com]
> > Sent: 2015?1?22? 17:31
> > To: Dudley Du
[...]
> > > struct gen5_bl_metadata_row_params {
> > > __le16 size;
> > > -__le16 maximun_size;
> > > +__le16 maximum_size;
> > I don't think sparse cares about variable names.
> > Be sure to note these changes in your log.
>
> This is a spell mistake issue, not sparse warning issue.
> Thanks.
>
That is fine but just be sure to note that in the log. I get suspicious
when there are changes that aren't mentioned in the log.
[...]
> > > +
> > > +if (app_integrity->flash_array_id != 0x00 ||
> > > +get_unaligned_be16(_integrity->row_number) != 0x01ff) {
> > I could be wrong but it looks there are a lot of changes un-related to
> > the sparse warning. I am not opposed to this per se but they should be
> > noted in the log message.
>
> Understood.
> Indeed, excepted the spell error, the purpose of the changes are aimed to fix
> the warning issue.
> Do I need to add the log message and re-submit the patch or in other method
> ot change this?
> Thanks.
>
Once I was able to reproduce the error there were quite a few. So this
number of changes seems reasonable.
The fix is already in -next so I don't think it is a big enough problem
to worry about.
--
- Jeremiah Mahler
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] input: cyapa: fix sparse warning issue of incorrect type in assiggment
On Thu, Jan 22, 2015 at 12:54:53PM +0300, Dan Carpenter wrote: > On Thu, Jan 22, 2015 at 01:31:29AM -0800, Jeremiah Mahler wrote: > > I wasn't able to reproduce the sparse warning. Was it for a specific > > architecture? Perhaps include a link to the email sent by the kbuild > > test robot. > > You need to add a "CF=-D__CHECK_ENDIAN__" to turn these on. > > make C=2 CF=-D__CHECK_ENDIAN__ > > http://lwn.net/Articles/205624/ > > regards, > dan carpenter > Even with __CHECK_ENDIAN__ I wasn't able to reproduce these errors straight away because this fix is already in next. But once reverted they appeared and the patch does fix all of them. jeri@hudson:~/linux-next$ git checkout 2be7256fe7f jeri@hudson:~/linux-next$ git reset --hard HEAD~1 jeri@hudson:~/linux-next$ make C=1 "CF=-D__CHECK_ENDIAN__" (errors ...) -- - Jeremiah Mahler -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] input: cyapa: fix sparse warning issue of incorrect type in assiggment
On Thu, Jan 22, 2015 at 12:54:53PM +0300, Dan Carpenter wrote: On Thu, Jan 22, 2015 at 01:31:29AM -0800, Jeremiah Mahler wrote: I wasn't able to reproduce the sparse warning. Was it for a specific architecture? Perhaps include a link to the email sent by the kbuild test robot. You need to add a CF=-D__CHECK_ENDIAN__ to turn these on. make C=2 CF=-D__CHECK_ENDIAN__ http://lwn.net/Articles/205624/ regards, dan carpenter Even with __CHECK_ENDIAN__ I wasn't able to reproduce these errors straight away because this fix is already in next. But once reverted they appeared and the patch does fix all of them. jeri@hudson:~/linux-next$ git checkout 2be7256fe7f jeri@hudson:~/linux-next$ git reset --hard HEAD~1 jeri@hudson:~/linux-next$ make C=1 CF=-D__CHECK_ENDIAN__ (errors ...) -- - Jeremiah Mahler -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] input: cyapa: fix sparse warning issue of incorrect type in assiggment
Dudley,
On Thu, Jan 22, 2015 at 09:57:42AM +, Dudley Du wrote:
-Original Message-
From: Jeremiah Mahler [mailto:jmmah...@gmail.com]
Sent: 2015?1?22? 17:31
To: Dudley Du
[...]
struct gen5_bl_metadata_row_params {
__le16 size;
-__le16 maximun_size;
+__le16 maximum_size;
I don't think sparse cares about variable names.
Be sure to note these changes in your log.
This is a spell mistake issue, not sparse warning issue.
Thanks.
That is fine but just be sure to note that in the log. I get suspicious
when there are changes that aren't mentioned in the log.
[...]
+
+if (app_integrity-flash_array_id != 0x00 ||
+get_unaligned_be16(app_integrity-row_number) != 0x01ff) {
I could be wrong but it looks there are a lot of changes un-related to
the sparse warning. I am not opposed to this per se but they should be
noted in the log message.
Understood.
Indeed, excepted the spell error, the purpose of the changes are aimed to fix
the warning issue.
Do I need to add the log message and re-submit the patch or in other method
ot change this?
Thanks.
Once I was able to reproduce the error there were quite a few. So this
number of changes seems reasonable.
The fix is already in -next so I don't think it is a big enough problem
to worry about.
--
- Jeremiah Mahler
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/2] input: cyapa: fix sparse warning issue of incorrect type in assiggment
> -Original Message- > From: Dan Carpenter [mailto:dan.carpen...@oracle.com] > Sent: 2015?1?22? 17:55 > To: Jeremiah Mahler; Dudley Du; dmitry.torok...@gmail.com; > rydb...@euromail.se; ble...@google.com; linux-in...@vger.kernel.org; > linux-kernel@vger.kernel.org > Subject: Re: [PATCH 1/2] input: cyapa: fix sparse warning issue of incorrect > type in > assiggment > > On Thu, Jan 22, 2015 at 01:31:29AM -0800, Jeremiah Mahler wrote: > > I wasn't able to reproduce the sparse warning. Was it for a specific > > architecture? Perhaps include a link to the email sent by the kbuild > > test robot. > > You need to add a "CF=-D__CHECK_ENDIAN__" to turn these on. > > make C=2 CF=-D__CHECK_ENDIAN__ Yes, thanks. I verified it locally, no issue found currently. > > http://lwn.net/Articles/205624/ > > regards, > dan carpenter This message and any attachments may contain Cypress (or its subsidiaries) confidential information. If it has been received in error, please advise the sender and immediately delete this message. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/2] input: cyapa: fix sparse warning issue of incorrect type in assiggment
> -Original Message-
> From: Jeremiah Mahler [mailto:jmmah...@gmail.com]
> Sent: 2015?1?22? 17:31
> To: Dudley Du
> Cc: dmitry.torok...@gmail.com; rydb...@euromail.se; ble...@google.com;
> dan.carpen...@oracle.com; linux-in...@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 1/2] input: cyapa: fix sparse warning issue of incorrect
> type in
> assiggment
>
> Dudley,
>
> Mis-spelled 'assiggment' in Subject line.
Thanks for spotting it.
>
> On Thu, Jan 22, 2015 at 02:50:05PM +0800, Dudley Du wrote:
> > Fixes the sparse warning issue of the incorrect type in assignment which
> > found by the kbuild test robot.
> >
> I wasn't able to reproduce the sparse warning. Was it for a specific
> architecture? Perhaps include a link to the email sent by the kbuild
> test robot.
Thanks.
The kbuild test robot sends automat email about the warning.
Based on Dimtry's modification and help, I generated the patch.
and tested locally with the command 'make C=1 DF="-D__CHECK_ENDIAN__"',
not issue found currently.
>
> [...]
> > struct gen5_bl_metadata_row_params {
> > __le16 size;
> > -__le16 maximun_size;
> > +__le16 maximum_size;
> I don't think sparse cares about variable names.
> Be sure to note these changes in your log.
This is a spell mistake issue, not sparse warning issue.
Thanks.
>
> > __le32 app_start;
> > __le16 app_len;
> > __le16 app_crc;
> > @@ -1192,67 +1192,69 @@ static int cyapa_gen5_bl_enter(struct cyapa
> *cyapa)
> > static int cyapa_gen5_check_fw(struct cyapa *cyapa, const struct firmware
> *fw)
> > {
> > struct device *dev = >client->dev;
> > -struct gen5_bl_metadata_row_params metadata;
> > -struct cyapa_tsg_bin_image *image;
> > -int flash_records_count;
> > -u16 app_crc = 0;
> > -u16 app_integrity_crc = 0;
> > -u16 row_num;
> > -u8 *data;
> > +const struct cyapa_tsg_bin_image *image = (const void *)fw->data;
> > +const struct cyapa_tsg_bin_image_data_record *app_integrity;
> > +const struct gen5_bl_metadata_row_params *metadata;
> > +size_t flash_records_count;
> > +u32 fw_app_start, fw_upgrade_start;
> > +u16 fw_app_len, fw_upgrade_len;
> > +u16 app_crc;
> > +u16 app_integrity_crc;
> > int record_index;
> > int i;
> >
> > -image = (struct cyapa_tsg_bin_image *)fw->data;
> > flash_records_count = (fw->size -
> > sizeof(struct cyapa_tsg_bin_image_head)) /
> > sizeof(struct cyapa_tsg_bin_image_data_record);
> >
> > -/* APP_INTEGRITY row is always the last row block,
> > - * and the row id must be 0x01ff */
> > -row_num = get_unaligned_be16(
> > ->records[flash_records_count - 1].row_number);
> > -if (image->records[flash_records_count - 1].flash_array_id != 0x00 &&
> > -row_num != 0x01ff) {
> > +/*
> > + * APP_INTEGRITY row is always the last row block,
> > + * and the row id must be 0x01ff.
> > + */
> > +app_integrity = >records[flash_records_count - 1];
> > +
> > +if (app_integrity->flash_array_id != 0x00 ||
> > +get_unaligned_be16(_integrity->row_number) != 0x01ff) {
> I could be wrong but it looks there are a lot of changes un-related to
> the sparse warning. I am not opposed to this per se but they should be
> noted in the log message.
Understood.
Indeed, excepted the spell error, the purpose of the changes are aimed to fix
the warning issue.
Do I need to add the log message and re-submit the patch or in other method ot
change this?
Thanks.
>
> [...]
>
> --
> - Jeremiah Mahler
This message and any attachments may contain Cypress (or its subsidiaries)
confidential information. If it has been received in error, please advise the
sender and immediately delete this message.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] input: cyapa: fix sparse warning issue of incorrect type in assiggment
On Thu, Jan 22, 2015 at 01:31:29AM -0800, Jeremiah Mahler wrote: > I wasn't able to reproduce the sparse warning. Was it for a specific > architecture? Perhaps include a link to the email sent by the kbuild > test robot. You need to add a "CF=-D__CHECK_ENDIAN__" to turn these on. make C=2 CF=-D__CHECK_ENDIAN__ http://lwn.net/Articles/205624/ regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] input: cyapa: fix sparse warning issue of incorrect type in assiggment
Dudley,
Mis-spelled 'assiggment' in Subject line.
On Thu, Jan 22, 2015 at 02:50:05PM +0800, Dudley Du wrote:
> Fixes the sparse warning issue of the incorrect type in assignment which
> found by the kbuild test robot.
>
I wasn't able to reproduce the sparse warning. Was it for a specific
architecture? Perhaps include a link to the email sent by the kbuild
test robot.
[...]
> struct gen5_bl_metadata_row_params {
> __le16 size;
> - __le16 maximun_size;
> + __le16 maximum_size;
I don't think sparse cares about variable names.
Be sure to note these changes in your log.
> __le32 app_start;
> __le16 app_len;
> __le16 app_crc;
> @@ -1192,67 +1192,69 @@ static int cyapa_gen5_bl_enter(struct cyapa *cyapa)
> static int cyapa_gen5_check_fw(struct cyapa *cyapa, const struct firmware
> *fw)
> {
> struct device *dev = >client->dev;
> - struct gen5_bl_metadata_row_params metadata;
> - struct cyapa_tsg_bin_image *image;
> - int flash_records_count;
> - u16 app_crc = 0;
> - u16 app_integrity_crc = 0;
> - u16 row_num;
> - u8 *data;
> + const struct cyapa_tsg_bin_image *image = (const void *)fw->data;
> + const struct cyapa_tsg_bin_image_data_record *app_integrity;
> + const struct gen5_bl_metadata_row_params *metadata;
> + size_t flash_records_count;
> + u32 fw_app_start, fw_upgrade_start;
> + u16 fw_app_len, fw_upgrade_len;
> + u16 app_crc;
> + u16 app_integrity_crc;
> int record_index;
> int i;
>
> - image = (struct cyapa_tsg_bin_image *)fw->data;
> flash_records_count = (fw->size -
> sizeof(struct cyapa_tsg_bin_image_head)) /
> sizeof(struct cyapa_tsg_bin_image_data_record);
>
> - /* APP_INTEGRITY row is always the last row block,
> - * and the row id must be 0x01ff */
> - row_num = get_unaligned_be16(
> - >records[flash_records_count - 1].row_number);
> - if (image->records[flash_records_count - 1].flash_array_id != 0x00 &&
> - row_num != 0x01ff) {
> + /*
> + * APP_INTEGRITY row is always the last row block,
> + * and the row id must be 0x01ff.
> + */
> + app_integrity = >records[flash_records_count - 1];
> +
> + if (app_integrity->flash_array_id != 0x00 ||
> + get_unaligned_be16(_integrity->row_number) != 0x01ff) {
I could be wrong but it looks there are a lot of changes un-related to
the sparse warning. I am not opposed to this per se but they should be
noted in the log message.
[...]
--
- Jeremiah Mahler
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/2] input: cyapa: fix sparse warning issue of incorrect type in assiggment
-Original Message-
From: Jeremiah Mahler [mailto:jmmah...@gmail.com]
Sent: 2015?1?22? 17:31
To: Dudley Du
Cc: dmitry.torok...@gmail.com; rydb...@euromail.se; ble...@google.com;
dan.carpen...@oracle.com; linux-in...@vger.kernel.org;
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] input: cyapa: fix sparse warning issue of incorrect
type in
assiggment
Dudley,
Mis-spelled 'assiggment' in Subject line.
Thanks for spotting it.
On Thu, Jan 22, 2015 at 02:50:05PM +0800, Dudley Du wrote:
Fixes the sparse warning issue of the incorrect type in assignment which
found by the kbuild test robot.
I wasn't able to reproduce the sparse warning. Was it for a specific
architecture? Perhaps include a link to the email sent by the kbuild
test robot.
Thanks.
The kbuild test robot sends automat email about the warning.
Based on Dimtry's modification and help, I generated the patch.
and tested locally with the command 'make C=1 DF=-D__CHECK_ENDIAN__',
not issue found currently.
[...]
struct gen5_bl_metadata_row_params {
__le16 size;
-__le16 maximun_size;
+__le16 maximum_size;
I don't think sparse cares about variable names.
Be sure to note these changes in your log.
This is a spell mistake issue, not sparse warning issue.
Thanks.
__le32 app_start;
__le16 app_len;
__le16 app_crc;
@@ -1192,67 +1192,69 @@ static int cyapa_gen5_bl_enter(struct cyapa
*cyapa)
static int cyapa_gen5_check_fw(struct cyapa *cyapa, const struct firmware
*fw)
{
struct device *dev = cyapa-client-dev;
-struct gen5_bl_metadata_row_params metadata;
-struct cyapa_tsg_bin_image *image;
-int flash_records_count;
-u16 app_crc = 0;
-u16 app_integrity_crc = 0;
-u16 row_num;
-u8 *data;
+const struct cyapa_tsg_bin_image *image = (const void *)fw-data;
+const struct cyapa_tsg_bin_image_data_record *app_integrity;
+const struct gen5_bl_metadata_row_params *metadata;
+size_t flash_records_count;
+u32 fw_app_start, fw_upgrade_start;
+u16 fw_app_len, fw_upgrade_len;
+u16 app_crc;
+u16 app_integrity_crc;
int record_index;
int i;
-image = (struct cyapa_tsg_bin_image *)fw-data;
flash_records_count = (fw-size -
sizeof(struct cyapa_tsg_bin_image_head)) /
sizeof(struct cyapa_tsg_bin_image_data_record);
-/* APP_INTEGRITY row is always the last row block,
- * and the row id must be 0x01ff */
-row_num = get_unaligned_be16(
-image-records[flash_records_count - 1].row_number);
-if (image-records[flash_records_count - 1].flash_array_id != 0x00
-row_num != 0x01ff) {
+/*
+ * APP_INTEGRITY row is always the last row block,
+ * and the row id must be 0x01ff.
+ */
+app_integrity = image-records[flash_records_count - 1];
+
+if (app_integrity-flash_array_id != 0x00 ||
+get_unaligned_be16(app_integrity-row_number) != 0x01ff) {
I could be wrong but it looks there are a lot of changes un-related to
the sparse warning. I am not opposed to this per se but they should be
noted in the log message.
Understood.
Indeed, excepted the spell error, the purpose of the changes are aimed to fix
the warning issue.
Do I need to add the log message and re-submit the patch or in other method ot
change this?
Thanks.
[...]
--
- Jeremiah Mahler
This message and any attachments may contain Cypress (or its subsidiaries)
confidential information. If it has been received in error, please advise the
sender and immediately delete this message.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/2] input: cyapa: fix sparse warning issue of incorrect type in assiggment
-Original Message- From: Dan Carpenter [mailto:dan.carpen...@oracle.com] Sent: 2015?1?22? 17:55 To: Jeremiah Mahler; Dudley Du; dmitry.torok...@gmail.com; rydb...@euromail.se; ble...@google.com; linux-in...@vger.kernel.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/2] input: cyapa: fix sparse warning issue of incorrect type in assiggment On Thu, Jan 22, 2015 at 01:31:29AM -0800, Jeremiah Mahler wrote: I wasn't able to reproduce the sparse warning. Was it for a specific architecture? Perhaps include a link to the email sent by the kbuild test robot. You need to add a CF=-D__CHECK_ENDIAN__ to turn these on. make C=2 CF=-D__CHECK_ENDIAN__ Yes, thanks. I verified it locally, no issue found currently. http://lwn.net/Articles/205624/ regards, dan carpenter This message and any attachments may contain Cypress (or its subsidiaries) confidential information. If it has been received in error, please advise the sender and immediately delete this message. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] input: cyapa: fix sparse warning issue of incorrect type in assiggment
Dudley,
Mis-spelled 'assiggment' in Subject line.
On Thu, Jan 22, 2015 at 02:50:05PM +0800, Dudley Du wrote:
Fixes the sparse warning issue of the incorrect type in assignment which
found by the kbuild test robot.
I wasn't able to reproduce the sparse warning. Was it for a specific
architecture? Perhaps include a link to the email sent by the kbuild
test robot.
[...]
struct gen5_bl_metadata_row_params {
__le16 size;
- __le16 maximun_size;
+ __le16 maximum_size;
I don't think sparse cares about variable names.
Be sure to note these changes in your log.
__le32 app_start;
__le16 app_len;
__le16 app_crc;
@@ -1192,67 +1192,69 @@ static int cyapa_gen5_bl_enter(struct cyapa *cyapa)
static int cyapa_gen5_check_fw(struct cyapa *cyapa, const struct firmware
*fw)
{
struct device *dev = cyapa-client-dev;
- struct gen5_bl_metadata_row_params metadata;
- struct cyapa_tsg_bin_image *image;
- int flash_records_count;
- u16 app_crc = 0;
- u16 app_integrity_crc = 0;
- u16 row_num;
- u8 *data;
+ const struct cyapa_tsg_bin_image *image = (const void *)fw-data;
+ const struct cyapa_tsg_bin_image_data_record *app_integrity;
+ const struct gen5_bl_metadata_row_params *metadata;
+ size_t flash_records_count;
+ u32 fw_app_start, fw_upgrade_start;
+ u16 fw_app_len, fw_upgrade_len;
+ u16 app_crc;
+ u16 app_integrity_crc;
int record_index;
int i;
- image = (struct cyapa_tsg_bin_image *)fw-data;
flash_records_count = (fw-size -
sizeof(struct cyapa_tsg_bin_image_head)) /
sizeof(struct cyapa_tsg_bin_image_data_record);
- /* APP_INTEGRITY row is always the last row block,
- * and the row id must be 0x01ff */
- row_num = get_unaligned_be16(
- image-records[flash_records_count - 1].row_number);
- if (image-records[flash_records_count - 1].flash_array_id != 0x00
- row_num != 0x01ff) {
+ /*
+ * APP_INTEGRITY row is always the last row block,
+ * and the row id must be 0x01ff.
+ */
+ app_integrity = image-records[flash_records_count - 1];
+
+ if (app_integrity-flash_array_id != 0x00 ||
+ get_unaligned_be16(app_integrity-row_number) != 0x01ff) {
I could be wrong but it looks there are a lot of changes un-related to
the sparse warning. I am not opposed to this per se but they should be
noted in the log message.
[...]
--
- Jeremiah Mahler
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] input: cyapa: fix sparse warning issue of incorrect type in assiggment
On Thu, Jan 22, 2015 at 01:31:29AM -0800, Jeremiah Mahler wrote: I wasn't able to reproduce the sparse warning. Was it for a specific architecture? Perhaps include a link to the email sent by the kbuild test robot. You need to add a CF=-D__CHECK_ENDIAN__ to turn these on. make C=2 CF=-D__CHECK_ENDIAN__ http://lwn.net/Articles/205624/ regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
