Re: [RFC PATCH 09/16] soundwire: crc8: add constant table

[Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>]

On 12/18/23 06:01, Vinod Koul wrote:
> On 07-12-23, 16:29, Pierre-Louis Bossart wrote:
>> Add the lookup table required by crc8(). All configuration values were
>> directly table from the MIPI SoundWire 1.x specification.
>>
>> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
>> ---
>>  drivers/soundwire/Makefile |   4 +-
>>  drivers/soundwire/crc8.c   | 277 +++++++++++++++++++++++++++++++++++++
>>  drivers/soundwire/crc8.h   |  11 ++
>>  3 files changed, 291 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/soundwire/crc8.c
>>  create mode 100644 drivers/soundwire/crc8.h
>>
>> diff --git a/drivers/soundwire/Makefile b/drivers/soundwire/Makefile
>> index 657f5888a77b..170128dd9318 100644
>> --- a/drivers/soundwire/Makefile
>> +++ b/drivers/soundwire/Makefile
>> @@ -5,7 +5,9 @@
>>  
>>  #Bus Objs
>>  soundwire-bus-y := bus_type.o bus.o master.o slave.o mipi_disco.o stream.o  \
>> -			sysfs_slave.o sysfs_slave_dpn.o
>> +			sysfs_slave.o sysfs_slave_dpn.o \
>> +			crc8.o
>> +
>>  obj-$(CONFIG_SOUNDWIRE) += soundwire-bus.o
>>  
>>  soundwire-generic-allocation-objs := generic_bandwidth_allocation.o
>> diff --git a/drivers/soundwire/crc8.c b/drivers/soundwire/crc8.c
>> new file mode 100644
>> index 000000000000..b6b984d7f39a
>> --- /dev/null
>> +++ b/drivers/soundwire/crc8.c
>> @@ -0,0 +1,277 @@
>> +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
>> +// Copyright(c) 2024 Intel Corporation.
>> +
>> +#include <linux/crc8.h>
>> +#include <linux/module.h>
>> +#include "crc8.h"
>> +
>> +/*
>> + * the MIPI SoundWire CRC8 polynomial is X^8 + X^6 + X^3 + X^2 + 1, MSB first
>> + * The value is (1)01001101 = 0x4D
>> + *
>> + * the table below was generated with
>> + *
>> + *	u8 crc8_lookup_table[CRC8_TABLE_SIZE];
>> + *	crc8_populate_msb(crc8_lookup_table, SDW_CRC8_POLY);
> 
> Good that you found this API, so next question would be why should we
> have this static table in kernel and not generate on probe if bpt is
> supported..? Many subsystems use these APIs to generate the tables..

The table is going to be the same for all hosts, it's simpler if
everyone uses a constant table, no? We're talking about 256 bytes added
for the common bus parts, be it with dynamically allocated memory or a
static table.

I don't mind reverting to a dynamically allocated table populated at
boot if that was the consensus.