* [PATCH v2 1/1] staging: iio: adc: ad7280a: use crc8.h API to build crc table
@ 2018-10-18 11:49 Slawomir Stepien
2018-10-18 17:31 ` Lars-Peter Clausen
2018-10-18 18:09 ` Slawomir Stepien
0 siblings, 2 replies; 5+ messages in thread
From: Slawomir Stepien @ 2018-10-18 11:49 UTC (permalink / raw)
To: lars, Michael.Hennerich, jic23, knaack.h, pmeerw; +Cc: linux-iio, gregkh
The custom build function ad7280_crc8_build_table is not needed. The
crc8_populate_msb function from linux/crc8.h will build the same crc
table.
Signed-off-by: Slawomir Stepien <sst@poczta.fm>
---
Since v1:
* Fix typo in commit message.
---
drivers/staging/iio/adc/ad7280a.c | 24 +++---------------------
1 file changed, 3 insertions(+), 21 deletions(-)
diff --git a/drivers/staging/iio/adc/ad7280a.c b/drivers/staging/iio/adc/ad7280a.c
index b736275c10f5..c701f07853fa 100644
--- a/drivers/staging/iio/adc/ad7280a.c
+++ b/drivers/staging/iio/adc/ad7280a.c
@@ -6,6 +6,7 @@
* Licensed under the GPL-2.
*/
+#include <linux/crc8.h>
#include <linux/device.h>
#include <linux/kernel.h>
#include <linux/slab.h>
@@ -121,8 +122,6 @@ static unsigned int ad7280a_devaddr(unsigned int addr)
* P(x) = x^8 + x^5 + x^3 + x^2 + x^1 + x^0 = 0b100101111 => 0x2F
*/
#define POLYNOM 0x2F
-#define POLYNOM_ORDER 8
-#define HIGHBIT (1 << (POLYNOM_ORDER - 1))
struct ad7280_state {
struct spi_device *spi;
@@ -131,7 +130,7 @@ struct ad7280_state {
int slave_num;
int scan_cnt;
int readback_delay_us;
- unsigned char crc_tab[256];
+ unsigned char crc_tab[CRC8_TABLE_SIZE];
unsigned char ctrl_hb;
unsigned char ctrl_lb;
unsigned char cell_threshhigh;
@@ -144,23 +143,6 @@ struct ad7280_state {
__be32 buf[2] ____cacheline_aligned;
};
-static void ad7280_crc8_build_table(unsigned char *crc_tab)
-{
- unsigned char bit, crc;
- int cnt, i;
-
- for (cnt = 0; cnt < 256; cnt++) {
- crc = cnt;
- for (i = 0; i < 8; i++) {
- bit = crc & HIGHBIT;
- crc <<= 1;
- if (bit)
- crc ^= POLYNOM;
- }
- crc_tab[cnt] = crc;
- }
-}
-
static unsigned char ad7280_calc_crc8(unsigned char *crc_tab, unsigned int val)
{
unsigned char crc;
@@ -857,7 +839,7 @@ static int ad7280_probe(struct spi_device *spi)
if (!pdata)
pdata = &ad7793_default_pdata;
- ad7280_crc8_build_table(st->crc_tab);
+ crc8_populate_msb(st->crc_tab, POLYNOM);
st->spi->max_speed_hz = AD7280A_MAX_SPI_CLK_HZ;
st->spi->mode = SPI_MODE_1;
--
2.19.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/1] staging: iio: adc: ad7280a: use crc8.h API to build crc table
2018-10-18 11:49 [PATCH v2 1/1] staging: iio: adc: ad7280a: use crc8.h API to build crc table Slawomir Stepien
@ 2018-10-18 17:31 ` Lars-Peter Clausen
2018-10-18 18:06 ` Slawomir Stepien
2018-10-18 18:09 ` Slawomir Stepien
1 sibling, 1 reply; 5+ messages in thread
From: Lars-Peter Clausen @ 2018-10-18 17:31 UTC (permalink / raw)
To: Slawomir Stepien, Michael.Hennerich, jic23, knaack.h, pmeerw
Cc: linux-iio, gregkh
On 10/18/2018 01:49 PM, Slawomir Stepien wrote:
> The custom build function ad7280_crc8_build_table is not needed. The
> crc8_populate_msb function from linux/crc8.h will build the same crc
> table.
>
> Signed-off-by: Slawomir Stepien <sst@poczta.fm>
This looks good, thanks.
Acked-by: Lars-Peter Clausen <lars@metafoo.de>
I supposed the code could be improved a bit further in a follow up patch by
only having one global table since it will be the same for all instances.
> ---
> Since v1:
> * Fix typo in commit message.
> ---
> drivers/staging/iio/adc/ad7280a.c | 24 +++---------------------
> 1 file changed, 3 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/ad7280a.c b/drivers/staging/iio/adc/ad7280a.c
> index b736275c10f5..c701f07853fa 100644
> --- a/drivers/staging/iio/adc/ad7280a.c
> +++ b/drivers/staging/iio/adc/ad7280a.c
> @@ -6,6 +6,7 @@
> * Licensed under the GPL-2.
> */
>
> +#include <linux/crc8.h>
> #include <linux/device.h>
> #include <linux/kernel.h>
> #include <linux/slab.h>
> @@ -121,8 +122,6 @@ static unsigned int ad7280a_devaddr(unsigned int addr)
> * P(x) = x^8 + x^5 + x^3 + x^2 + x^1 + x^0 = 0b100101111 => 0x2F
> */
> #define POLYNOM 0x2F
> -#define POLYNOM_ORDER 8
> -#define HIGHBIT (1 << (POLYNOM_ORDER - 1))
>
> struct ad7280_state {
> struct spi_device *spi;
> @@ -131,7 +130,7 @@ struct ad7280_state {
> int slave_num;
> int scan_cnt;
> int readback_delay_us;
> - unsigned char crc_tab[256];
> + unsigned char crc_tab[CRC8_TABLE_SIZE];
> unsigned char ctrl_hb;
> unsigned char ctrl_lb;
> unsigned char cell_threshhigh;
> @@ -144,23 +143,6 @@ struct ad7280_state {
> __be32 buf[2] ____cacheline_aligned;
> };
>
> -static void ad7280_crc8_build_table(unsigned char *crc_tab)
> -{
> - unsigned char bit, crc;
> - int cnt, i;
> -
> - for (cnt = 0; cnt < 256; cnt++) {
> - crc = cnt;
> - for (i = 0; i < 8; i++) {
> - bit = crc & HIGHBIT;
> - crc <<= 1;
> - if (bit)
> - crc ^= POLYNOM;
> - }
> - crc_tab[cnt] = crc;
> - }
> -}
> -
> static unsigned char ad7280_calc_crc8(unsigned char *crc_tab, unsigned int val)
> {
> unsigned char crc;
> @@ -857,7 +839,7 @@ static int ad7280_probe(struct spi_device *spi)
> if (!pdata)
> pdata = &ad7793_default_pdata;
>
> - ad7280_crc8_build_table(st->crc_tab);
> + crc8_populate_msb(st->crc_tab, POLYNOM);
>
> st->spi->max_speed_hz = AD7280A_MAX_SPI_CLK_HZ;
> st->spi->mode = SPI_MODE_1;
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/1] staging: iio: adc: ad7280a: use crc8.h API to build crc table
2018-10-18 17:31 ` Lars-Peter Clausen
@ 2018-10-18 18:06 ` Slawomir Stepien
2018-10-18 18:08 ` Lars-Peter Clausen
0 siblings, 1 reply; 5+ messages in thread
From: Slawomir Stepien @ 2018-10-18 18:06 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Michael.Hennerich, jic23, knaack.h, pmeerw, linux-iio, gregkh
On paź 18, 2018 19:31, Lars-Peter Clausen wrote:
> On 10/18/2018 01:49 PM, Slawomir Stepien wrote:
> > The custom build function ad7280_crc8_build_table is not needed. The
> > crc8_populate_msb function from linux/crc8.h will build the same crc
> > table.
> >
> > Signed-off-by: Slawomir Stepien <sst@poczta.fm>
>
> This looks good, thanks.
>
> Acked-by: Lars-Peter Clausen <lars@metafoo.de>
Thank you.
> I supposed the code could be improved a bit further in a follow up patch by
> only having one global table since it will be the same for all instances.
Yes that's a good idea.
How should I proceed? v3 or a separate commit based on the changes in this
commit? I think separate commit will be better, so I can make a good commit
message...or maybe put it all in a patch series with your ack added to this
change? Sorry I don't know the rules in such cases.
--
Slawomir Stepien
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/1] staging: iio: adc: ad7280a: use crc8.h API to build crc table
2018-10-18 18:06 ` Slawomir Stepien
@ 2018-10-18 18:08 ` Lars-Peter Clausen
0 siblings, 0 replies; 5+ messages in thread
From: Lars-Peter Clausen @ 2018-10-18 18:08 UTC (permalink / raw)
To: Slawomir Stepien
Cc: Michael.Hennerich, jic23, knaack.h, pmeerw, linux-iio, gregkh
On 10/18/2018 08:06 PM, Slawomir Stepien wrote:
> On paź 18, 2018 19:31, Lars-Peter Clausen wrote:
>> On 10/18/2018 01:49 PM, Slawomir Stepien wrote:
>>> The custom build function ad7280_crc8_build_table is not needed. The
>>> crc8_populate_msb function from linux/crc8.h will build the same crc
>>> table.
>>>
>>> Signed-off-by: Slawomir Stepien <sst@poczta.fm>
>>
>> This looks good, thanks.
>>
>> Acked-by: Lars-Peter Clausen <lars@metafoo.de>
>
> Thank you.
>
>> I supposed the code could be improved a bit further in a follow up patch by
>> only having one global table since it will be the same for all instances.
>
> Yes that's a good idea.
> How should I proceed? v3 or a separate commit based on the changes in this
> commit? I think separate commit will be better, so I can make a good commit
> message...or maybe put it all in a patch series with your ack added to this
> change? Sorry I don't know the rules in such cases.
I think Jonathan will pick this patch up soon. I'd wait until the patch has
been applied and then send your next patch with further improvements to the
driver.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/1] staging: iio: adc: ad7280a: use crc8.h API to build crc table
2018-10-18 11:49 [PATCH v2 1/1] staging: iio: adc: ad7280a: use crc8.h API to build crc table Slawomir Stepien
2018-10-18 17:31 ` Lars-Peter Clausen
@ 2018-10-18 18:09 ` Slawomir Stepien
1 sibling, 0 replies; 5+ messages in thread
From: Slawomir Stepien @ 2018-10-18 18:09 UTC (permalink / raw)
To: lars, Michael.Hennerich, jic23, knaack.h, pmeerw; +Cc: linux-iio, gregkh
On paź 18, 2018 13:49, Slawomir Stepien wrote:
> The custom build function ad7280_crc8_build_table is not needed. The
> crc8_populate_msb function from linux/crc8.h will build the same crc
> table.
>
> Signed-off-by: Slawomir Stepien <sst@poczta.fm>
> ---
> Since v1:
> * Fix typo in commit message.
> ---
> drivers/staging/iio/adc/ad7280a.c | 24 +++---------------------
> 1 file changed, 3 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/ad7280a.c b/drivers/staging/iio/adc/ad7280a.c
> index b736275c10f5..c701f07853fa 100644
> --- a/drivers/staging/iio/adc/ad7280a.c
> +++ b/drivers/staging/iio/adc/ad7280a.c
> @@ -6,6 +6,7 @@
> * Licensed under the GPL-2.
> */
>
> +#include <linux/crc8.h>
> #include <linux/device.h>
> #include <linux/kernel.h>
> #include <linux/slab.h>
> @@ -121,8 +122,6 @@ static unsigned int ad7280a_devaddr(unsigned int addr)
> * P(x) = x^8 + x^5 + x^3 + x^2 + x^1 + x^0 = 0b100101111 => 0x2F
> */
> #define POLYNOM 0x2F
> -#define POLYNOM_ORDER 8
> -#define HIGHBIT (1 << (POLYNOM_ORDER - 1))
>
> struct ad7280_state {
> struct spi_device *spi;
> @@ -131,7 +130,7 @@ struct ad7280_state {
> int slave_num;
> int scan_cnt;
> int readback_delay_us;
> - unsigned char crc_tab[256];
> + unsigned char crc_tab[CRC8_TABLE_SIZE];
> unsigned char ctrl_hb;
> unsigned char ctrl_lb;
> unsigned char cell_threshhigh;
> @@ -144,23 +143,6 @@ struct ad7280_state {
> __be32 buf[2] ____cacheline_aligned;
> };
>
> -static void ad7280_crc8_build_table(unsigned char *crc_tab)
> -{
> - unsigned char bit, crc;
> - int cnt, i;
> -
> - for (cnt = 0; cnt < 256; cnt++) {
> - crc = cnt;
> - for (i = 0; i < 8; i++) {
> - bit = crc & HIGHBIT;
> - crc <<= 1;
> - if (bit)
> - crc ^= POLYNOM;
> - }
> - crc_tab[cnt] = crc;
> - }
> -}
> -
> static unsigned char ad7280_calc_crc8(unsigned char *crc_tab, unsigned int val)
> {
> unsigned char crc;
> @@ -857,7 +839,7 @@ static int ad7280_probe(struct spi_device *spi)
> if (!pdata)
> pdata = &ad7793_default_pdata;
>
> - ad7280_crc8_build_table(st->crc_tab);
> + crc8_populate_msb(st->crc_tab, POLYNOM);
Ups...this function requires CONFIG_CRC8. I need to make a v3 with Kbuild
change.
> st->spi->max_speed_hz = AD7280A_MAX_SPI_CLK_HZ;
> st->spi->mode = SPI_MODE_1;
--
Slawomir Stepien
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-10-19 2:11 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-18 11:49 [PATCH v2 1/1] staging: iio: adc: ad7280a: use crc8.h API to build crc table Slawomir Stepien
2018-10-18 17:31 ` Lars-Peter Clausen
2018-10-18 18:06 ` Slawomir Stepien
2018-10-18 18:08 ` Lars-Peter Clausen
2018-10-18 18:09 ` Slawomir Stepien
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.