| Message ID | 20180207160002.GA9292@embeddedgus (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
"Gustavo A. R. Silva" <gustavo@embeddedor.com> writes: > Add suffix ULL to constant 9 in order to give the compiler complete > information about the proper arithmetic to use. Notice that this > constant is used in a context that expects an expression of type > unsigned long long (64 bits, unsigned). > > The expression tfr->len * 9 * 1000000 is currently being evaluated > using 32-bit arithmetic. > > Addresses-Coverity-ID: 1339619 > Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> The effect looks like it would be that we would have chosen polling mode instead of waiting for interrupts for some transfers >477 seconds. Seems like a good fix for an unlikely bug. Reviewed-by: Eric Anholt <eric@anholt.net>
On 7 February 2018 at 16:00, Gustavo A. R. Silva <gustavo@embeddedor.com> wrote: > Add suffix ULL to constant 9 in order to give the compiler complete > information about the proper arithmetic to use. Notice that this > constant is used in a context that expects an expression of type > unsigned long long (64 bits, unsigned). > > The expression tfr->len * 9 * 1000000 is currently being evaluated > using 32-bit arithmetic. > > Addresses-Coverity-ID: 1339619 What does this number mean? If it is an index into some internal database, please remove it. > Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> > --- > drivers/spi/spi-bcm2835aux.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/spi/spi-bcm2835aux.c b/drivers/spi/spi-bcm2835aux.c > index 7428091..a768c23 100644 > --- a/drivers/spi/spi-bcm2835aux.c > +++ b/drivers/spi/spi-bcm2835aux.c > @@ -363,7 +363,7 @@ static int bcm2835aux_spi_transfer_one(struct spi_master *master, > * chunk getting transferred - in our case the chunk size > * is 3 bytes, so we approximate this by 9 bits/byte > */ > - xfer_time_us = tfr->len * 9 * 1000000; > + xfer_time_us = tfr->len * 9ULL * 1000000; > do_div(xfer_time_us, spi_used_hz); > > /* run in polling mode for short transfers */ > -- > 2.7.4 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Eric, On 02/08/2018 02:22 AM, Eric Anholt wrote: > "Gustavo A. R. Silva" <gustavo@embeddedor.com> writes: > >> Add suffix ULL to constant 9 in order to give the compiler complete >> information about the proper arithmetic to use. Notice that this >> constant is used in a context that expects an expression of type >> unsigned long long (64 bits, unsigned). >> >> The expression tfr->len * 9 * 1000000 is currently being evaluated >> using 32-bit arithmetic. >> >> Addresses-Coverity-ID: 1339619 >> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> > > The effect looks like it would be that we would have chosen polling mode > instead of waiting for interrupts for some transfers >477 seconds. > Seems like a good fix for an unlikely bug. > > Reviewed-by: Eric Anholt <eric@anholt.net> > Thank you for your review. -- Gustavo -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ard, On 02/08/2018 03:54 AM, Ard Biesheuvel wrote: > On 7 February 2018 at 16:00, Gustavo A. R. Silva <gustavo@embeddedor.com> wrote: >> Add suffix ULL to constant 9 in order to give the compiler complete >> information about the proper arithmetic to use. Notice that this >> constant is used in a context that expects an expression of type >> unsigned long long (64 bits, unsigned). >> >> The expression tfr->len * 9 * 1000000 is currently being evaluated >> using 32-bit arithmetic. >> >> Addresses-Coverity-ID: 1339619 > > What does this number mean? If it is an index into some internal > database, please remove it. > This is a unique Coverity identifier. We want to keep information like public Bugzilla IDs and tools like Coverity on the commit message. Thanks -- Gustavo -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12 February 2018 at 18:04, Gustavo A. R. Silva <garsilva@embeddedor.com> wrote: > Hi Ard, > > On 02/08/2018 03:54 AM, Ard Biesheuvel wrote: >> >> On 7 February 2018 at 16:00, Gustavo A. R. Silva <gustavo@embeddedor.com> >> wrote: >>> >>> Add suffix ULL to constant 9 in order to give the compiler complete >>> information about the proper arithmetic to use. Notice that this >>> constant is used in a context that expects an expression of type >>> unsigned long long (64 bits, unsigned). >>> >>> The expression tfr->len * 9 * 1000000 is currently being evaluated >>> using 32-bit arithmetic. >>> >>> Addresses-Coverity-ID: 1339619 >> >> >> What does this number mean? If it is an index into some internal >> database, please remove it. >> > > This is a unique Coverity identifier. We want to keep information like > public Bugzilla IDs and tools like Coverity on the commit message. > Who is 'we' in this case? And how is this id to any benefit of other people that have been excluded from 'we'? If you add identifiers like this, make sure that they don't only make sense to the in-crowd. For instance, you could replace this with a http link to the database entry if you really must. -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/12/2018 10:45 AM, Ard Biesheuvel wrote: > On 12 February 2018 at 18:04, Gustavo A. R. Silva > <garsilva@embeddedor.com> wrote: >> Hi Ard, >> >> On 02/08/2018 03:54 AM, Ard Biesheuvel wrote: >>> >>> On 7 February 2018 at 16:00, Gustavo A. R. Silva <gustavo@embeddedor.com> >>> wrote: >>>> >>>> Add suffix ULL to constant 9 in order to give the compiler complete >>>> information about the proper arithmetic to use. Notice that this >>>> constant is used in a context that expects an expression of type >>>> unsigned long long (64 bits, unsigned). >>>> >>>> The expression tfr->len * 9 * 1000000 is currently being evaluated >>>> using 32-bit arithmetic. >>>> >>>> Addresses-Coverity-ID: 1339619 >>> >>> >>> What does this number mean? If it is an index into some internal >>> database, please remove it. >>> >> >> This is a unique Coverity identifier. We want to keep information like >> public Bugzilla IDs and tools like Coverity on the commit message. >> > > Who is 'we' in this case? And how is this id to any benefit of other > people that have been excluded from 'we'? We is probably the greater Linux community here. > > If you add identifiers like this, make sure that they don't only make > sense to the in-crowd. For instance, you could replace this with a > http link to the database entry if you really must. I don't think it is that easy to extract good URLs from the public Linux coverity instance which is why referring to coverity IDs is being done AFAICT.
On 12 February 2018 at 19:10, Florian Fainelli <f.fainelli@gmail.com> wrote: > On 02/12/2018 10:45 AM, Ard Biesheuvel wrote: >> On 12 February 2018 at 18:04, Gustavo A. R. Silva >> <garsilva@embeddedor.com> wrote: >>> Hi Ard, >>> >>> On 02/08/2018 03:54 AM, Ard Biesheuvel wrote: >>>> >>>> On 7 February 2018 at 16:00, Gustavo A. R. Silva <gustavo@embeddedor.com> >>>> wrote: >>>>> >>>>> Add suffix ULL to constant 9 in order to give the compiler complete >>>>> information about the proper arithmetic to use. Notice that this >>>>> constant is used in a context that expects an expression of type >>>>> unsigned long long (64 bits, unsigned). >>>>> >>>>> The expression tfr->len * 9 * 1000000 is currently being evaluated >>>>> using 32-bit arithmetic. >>>>> >>>>> Addresses-Coverity-ID: 1339619 >>>> >>>> >>>> What does this number mean? If it is an index into some internal >>>> database, please remove it. >>>> >>> >>> This is a unique Coverity identifier. We want to keep information like >>> public Bugzilla IDs and tools like Coverity on the commit message. >>> >> >> Who is 'we' in this case? And how is this id to any benefit of other >> people that have been excluded from 'we'? > > We is probably the greater Linux community here. > >> >> If you add identifiers like this, make sure that they don't only make >> sense to the in-crowd. For instance, you could replace this with a >> http link to the database entry if you really must. > > I don't think it is that easy to extract good URLs from the public Linux > coverity instance which is why referring to coverity IDs is being done > AFAICT. Ah ok, pardon my ignorance then. I wasn't aware there is a public Linux coverity instance. Got a link? -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/12/2018 11:11 AM, Ard Biesheuvel wrote: > On 12 February 2018 at 19:10, Florian Fainelli <f.fainelli@gmail.com> wrote: >> On 02/12/2018 10:45 AM, Ard Biesheuvel wrote: >>> On 12 February 2018 at 18:04, Gustavo A. R. Silva >>> <garsilva@embeddedor.com> wrote: >>>> Hi Ard, >>>> >>>> On 02/08/2018 03:54 AM, Ard Biesheuvel wrote: >>>>> >>>>> On 7 February 2018 at 16:00, Gustavo A. R. Silva <gustavo@embeddedor.com> >>>>> wrote: >>>>>> >>>>>> Add suffix ULL to constant 9 in order to give the compiler complete >>>>>> information about the proper arithmetic to use. Notice that this >>>>>> constant is used in a context that expects an expression of type >>>>>> unsigned long long (64 bits, unsigned). >>>>>> >>>>>> The expression tfr->len * 9 * 1000000 is currently being evaluated >>>>>> using 32-bit arithmetic. >>>>>> >>>>>> Addresses-Coverity-ID: 1339619 >>>>> >>>>> >>>>> What does this number mean? If it is an index into some internal >>>>> database, please remove it. >>>>> >>>> >>>> This is a unique Coverity identifier. We want to keep information like >>>> public Bugzilla IDs and tools like Coverity on the commit message. >>>> >>> >>> Who is 'we' in this case? And how is this id to any benefit of other >>> people that have been excluded from 'we'? >> >> We is probably the greater Linux community here. >> >>> >>> If you add identifiers like this, make sure that they don't only make >>> sense to the in-crowd. For instance, you could replace this with a >>> http link to the database entry if you really must. >> >> I don't think it is that easy to extract good URLs from the public Linux >> coverity instance which is why referring to coverity IDs is being done >> AFAICT. > > Ah ok, pardon my ignorance then. I wasn't aware there is a public > Linux coverity instance. Got a link? It requires you to sign up to get notifications and have a dashboard, does this link work for you? https://scan.coverity.com/projects/linux?tab=overview
T24gV2VkLCAyMDE4LTAyLTA3IGF0IDEwOjAwIC0wNjAwLCBHdXN0YXZvIEEuIFIuIFNpbHZhIHdy b3RlOg0KPiBBZGQgc3VmZml4IFVMTCB0byBjb25zdGFudCA5IGluIG9yZGVyIHRvIGdpdmUgdGhl IGNvbXBpbGVyIGNvbXBsZXRlDQo+IGluZm9ybWF0aW9uIGFib3V0IHRoZSBwcm9wZXIgYXJpdGht ZXRpYyB0byB1c2UuIE5vdGljZSB0aGF0IHRoaXMNCj4gY29uc3RhbnQgaXMgdXNlZCBpbiBhIGNv bnRleHQgdGhhdCBleHBlY3RzIGFuIGV4cHJlc3Npb24gb2YgdHlwZQ0KPiB1bnNpZ25lZCBsb25n IGxvbmcgKDY0IGJpdHMsIHVuc2lnbmVkKS4NCj4gDQo+IFRoZSBleHByZXNzaW9uIHRmci0+bGVu ICogOSAqIDEwMDAwMDAgaXMgY3VycmVudGx5IGJlaW5nIGV2YWx1YXRlZA0KPiB1c2luZyAzMi1i aXQgYXJpdGhtZXRpYy4NCj4gDQo+IEFkZHJlc3Nlcy1Db3Zlcml0eS1JRDogMTMzOTYxOQ0KPiBT aWduZWQtb2ZmLWJ5OiBHdXN0YXZvIEEuIFIuIFNpbHZhIDxndXN0YXZvQGVtYmVkZGVkb3IuY29t Pg0KPiAtLS0NCj4gIGRyaXZlcnMvc3BpL3NwaS1iY20yODM1YXV4LmMgfCAyICstDQo+ICAxIGZp bGUgY2hhbmdlZCwgMSBpbnNlcnRpb24oKyksIDEgZGVsZXRpb24oLSkNCj4gDQo+IGRpZmYgLS1n aXQgYS9kcml2ZXJzL3NwaS9zcGktYmNtMjgzNWF1eC5jIGIvZHJpdmVycy9zcGkvc3BpLWJjbTI4 MzVhdXguYw0KPiBpbmRleCA3NDI4MDkxLi5hNzY4YzIzIDEwMDY0NA0KPiAtLS0gYS9kcml2ZXJz L3NwaS9zcGktYmNtMjgzNWF1eC5jDQo+ICsrKyBiL2RyaXZlcnMvc3BpL3NwaS1iY20yODM1YXV4 LmMNCj4gQEAgLTM2Myw3ICszNjMsNyBAQCBzdGF0aWMgaW50IGJjbTI4MzVhdXhfc3BpX3RyYW5z ZmVyX29uZShzdHJ1Y3Qgc3BpX21hc3RlciAqbWFzdGVyLA0KPiAgCSAqIGNodW5rIGdldHRpbmcg dHJhbnNmZXJyZWQgLSBpbiBvdXIgY2FzZSB0aGUgY2h1bmsgc2l6ZQ0KPiAgCSAqIGlzIDMgYnl0 ZXMsIHNvIHdlIGFwcHJveGltYXRlIHRoaXMgYnkgOSBiaXRzL2J5dGUNCj4gIAkgKi8NCj4gLQl4 ZmVyX3RpbWVfdXMgPSB0ZnItPmxlbiAqIDkgKiAxMDAwMDAwOw0KPiArCXhmZXJfdGltZV91cyA9 IHRmci0+bGVuICogOVVMTCAqIDEwMDAwMDA7DQo+ICAJZG9fZGl2KHhmZXJfdGltZV91cywgc3Bp X3VzZWRfaHopOw0KPiAgDQo+ICAJLyogcnVuIGluIHBvbGxpbmcgbW9kZSBmb3Igc2hvcnQgdHJh bnNmZXJzICovDQoJaWYgKHhmZXJfdGltZV91cyA8IEJDTTI4MzVfQVVYX1NQSV9QT0xMSU5HX0xJ TUlUX1VTKQ0KDQpJZiBvbmUgbG9va3MgYXQgdGhlIGxvZ2ljIGFzIGEgd2hvbGUuLi4NCg0KdGZy LT5sZW4gKiA5VUxMICogMTAwMDAwIC8gc3BpX3VzZWRfaHogPCAzMA0KDQpBIGxpdHRsZSBhbGdl YnJhIGNhbiBjaGFuZ2UgdGhhdCBpbnRvOg0KDQp0ZnItPmxlbiA8IHNwaV91c2VkX2h6IC8gMzAw MDAwDQoNClNvIHJhdGhlciB0aGFuIGFkZCBtb3JlIDY0LWJpdCBtYXRoLCBpdCBjYW4gYmUgcmVt b3ZlZCBlbnRpcmVseS4gIEJvdGgNCnRoZSBsZW5ndGggYW5kIHNwZWVkIGFyZSAzMiBiaXRzIGFu ZCB0aGVyZSdzIG5vIG5lZWQgdG8gbXVsdGlwbHkgdGhlbQ0KaW50byBzb21ldGhpbmcgbGFyZ2Vy IHRoYW4gbWlnaHQgbmVlZCA2NCBiaXRzLg0K -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/spi/spi-bcm2835aux.c b/drivers/spi/spi-bcm2835aux.c index 7428091..a768c23 100644 --- a/drivers/spi/spi-bcm2835aux.c +++ b/drivers/spi/spi-bcm2835aux.c @@ -363,7 +363,7 @@ static int bcm2835aux_spi_transfer_one(struct spi_master *master, * chunk getting transferred - in our case the chunk size * is 3 bytes, so we approximate this by 9 bits/byte */ - xfer_time_us = tfr->len * 9 * 1000000; + xfer_time_us = tfr->len * 9ULL * 1000000; do_div(xfer_time_us, spi_used_hz); /* run in polling mode for short transfers */
Add suffix ULL to constant 9 in order to give the compiler complete information about the proper arithmetic to use. Notice that this constant is used in a context that expects an expression of type unsigned long long (64 bits, unsigned). The expression tfr->len * 9 * 1000000 is currently being evaluated using 32-bit arithmetic. Addresses-Coverity-ID: 1339619 Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> --- drivers/spi/spi-bcm2835aux.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)