On 12/21/23 15:42, Vinod Koul wrote: > On 18-12-23, 14:26, Pierre-Louis Bossart wrote: >> >> >> 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. > > Most of the kernel users I looked have dynamically allocated table > populated at boot, also out of many users how many would support BTP.? > Your older platforms, current qcom dont, so not point is adding for > everyone... All Intel hardware supports BTP/BRA, we just didn't have a compelling reason to enable it so far. I've seen AMD stating that they also have BTP/BRA. That's 2/3 of controllers. I don't mind reverting to a devm_ allocated table, I am not sure I see the benefits for 256 bytes of constant data.
