| Message ID | 20210510131357.17170-1-ckeepax@opensource.cirrus.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Tidy up device ID reading on legacy Cirrus parts | expand |
On 5/10/21 8:13 AM, Charles Keepax wrote: > Pierre requested I have a look at some cppcheck warnings in the cs42l42 > driver, since it is reassigning the ret variable without ever checking > the result. Looking a bit more broadly this happens in quite a few > legacy Cirrus parts, as they all use the same process to read the ID, > factor out a small helper so they can all share the same code. Whilst > in there fix up a couple of other trivial error path issues as well. > > Thanks, > Charles > > Charles Keepax (10): > ASoC: cirrus: Add helper function for reading the device ID > ASoC: cs35l32: Minor error paths fixups > ASoC: cs35l33: Minor error paths fixups > ASoC: cs35l34: Minor error paths fixups > ASoC: cs35l35: Minor error paths fixups > ASoC: cs35l35: Correct errata handling > ASoC: cs42l42: Minor error paths fixups > ASoC: cs42l73: Minor error paths fixups > ASoC: cs43130: Minor error paths fixups > ASoC: cs53l30: Minor error paths fixups Sounds all good to me, just wondering if while you're at it you can fix the remaining minor issues? Thanks! cppcheck --platform=unix64 --force --max-configs=1024 --inconclusive --enable=all --suppress=variableScope --suppress=shiftTooManyBitsSigned --suppress=arithOperationsOnVoidPointer --suppress=bitwiseOnBoolean sound/soc/codecs/cs*.c sound/soc/codecs/cs35l36.c:1159:10: style: Variable 'ret' is assigned a value that is never used. [unreadVariable] int ret = 0; ^ sound/soc/codecs/cs4265.c:619:6: style: Variable 'ret' is reassigned a value before the old one has been used. [redundantAssignment] ret = devm_snd_soc_register_component(&i2c_client->dev, ^ sound/soc/codecs/cs4265.c:604:6: note: ret is assigned ret = regmap_read(cs4265->regmap, CS4265_CHIP_ID, ®); ^ sound/soc/codecs/cs4265.c:619:6: note: ret is overwritten ret = devm_snd_soc_register_component(&i2c_client->dev, ^ sound/soc/codecs/cs42l52.c:1202:6: style: Variable 'ret' is reassigned a value before the old one has been used. [redundantAssignment] ret = devm_snd_soc_register_component(&i2c_client->dev, ^ sound/soc/codecs/cs42l52.c:1165:6: note: ret is assigned ret = regmap_read(cs42l52->regmap, CS42L52_CHIP, ®); ^ sound/soc/codecs/cs42l52.c:1202:6: note: ret is overwritten ret = devm_snd_soc_register_component(&i2c_client->dev, ^ > > sound/soc/codecs/cirrus_legacy.h | 21 +++++++++++++++++++++ > sound/soc/codecs/cs35l32.c | 34 ++++++++++++++++++---------------- > sound/soc/codecs/cs35l33.c | 15 +++++++++------ > sound/soc/codecs/cs35l34.c | 39 ++++++++++++++++++++++----------------- > sound/soc/codecs/cs35l35.c | 21 ++++++++++----------- > sound/soc/codecs/cs35l35.h | 1 + > sound/soc/codecs/cs42l42.c | 18 ++++++++---------- > sound/soc/codecs/cs42l73.c | 30 +++++++++++++++++------------- > sound/soc/codecs/cs43130.c | 31 +++++++++++++++++++------------ > sound/soc/codecs/cs53l30.c | 22 +++++++++++----------- > 10 files changed, 136 insertions(+), 96 deletions(-) > create mode 100644 sound/soc/codecs/cirrus_legacy.h >
On Mon, 10 May 2021 14:13:47 +0100, Charles Keepax wrote: > Pierre requested I have a look at some cppcheck warnings in the cs42l42 > driver, since it is reassigning the ret variable without ever checking > the result. Looking a bit more broadly this happens in quite a few > legacy Cirrus parts, as they all use the same process to read the ID, > factor out a small helper so they can all share the same code. Whilst > in there fix up a couple of other trivial error path issues as well. > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [01/10] ASoC: cirrus: Add helper function for reading the device ID commit: c8b198ed31000a48f507bcea3828374b75418a2f [02/10] ASoC: cs35l32: Minor error paths fixups commit: 283160f1419ddebc8779c3488e800cd30b31289d [03/10] ASoC: cs35l33: Minor error paths fixups commit: 77908dbecdb6be2e875ced738b5c036bb83e8d78 [04/10] ASoC: cs35l34: Minor error paths fixups commit: 8cb9b001635cfa0389ec54eb511cb459968ad1d7 [05/10] ASoC: cs35l35: Minor error paths fixups commit: 60ba916d87600684a1e127b484e1c407c355caad [06/10] ASoC: cs35l35: Correct errata handling commit: 1a46b7b82df57b6b6a4e891cdbb2de1cf818a43b [07/10] ASoC: cs42l42: Minor error paths fixups commit: 0a0eb567e1d43cb87e9740c8e417d6fcff061582 [08/10] ASoC: cs42l73: Minor error paths fixups commit: 26495252fe0d1ebf548c02cb63b51abae5d5e5a3 [09/10] ASoC: cs43130: Minor error paths fixups commit: e2bb1077cee4d13dc85d53d76deac73b24d7f845 [10/10] ASoC: cs53l30: Minor error paths fixups commit: 4fc81bc88ad9d6bac90d169382c6045c47d48648 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
On Mon, May 10, 2021 at 09:59:55AM -0500, Pierre-Louis Bossart wrote: > On 5/10/21 8:13 AM, Charles Keepax wrote: > Sounds all good to me, just wondering if while you're at it you can > fix the remaining minor issues? Thanks! > > cppcheck --platform=unix64 --force --max-configs=1024 --inconclusive > --enable=all --suppress=variableScope > --suppress=shiftTooManyBitsSigned > --suppress=arithOperationsOnVoidPointer --suppress=bitwiseOnBoolean > sound/soc/codecs/cs*.c > > sound/soc/codecs/cs35l36.c:1159:10: style: Variable 'ret' is > assigned a value that is never used. [unreadVariable] > int ret = 0; > ^ > sound/soc/codecs/cs4265.c:619:6: style: Variable 'ret' is reassigned > a value before the old one has been used. [redundantAssignment] > ret = devm_snd_soc_register_component(&i2c_client->dev, > ^ > sound/soc/codecs/cs4265.c:604:6: note: ret is assigned > ret = regmap_read(cs4265->regmap, CS4265_CHIP_ID, ®); > ^ > sound/soc/codecs/cs4265.c:619:6: note: ret is overwritten > ret = devm_snd_soc_register_component(&i2c_client->dev, > ^ > sound/soc/codecs/cs42l52.c:1202:6: style: Variable 'ret' is > reassigned a value before the old one has been used. > [redundantAssignment] > ret = devm_snd_soc_register_component(&i2c_client->dev, > ^ > sound/soc/codecs/cs42l52.c:1165:6: note: ret is assigned > ret = regmap_read(cs42l52->regmap, CS42L52_CHIP, ®); > ^ > sound/soc/codecs/cs42l52.c:1202:6: note: ret is overwritten > ret = devm_snd_soc_register_component(&i2c_client->dev, Apologies my cppcheck doesn't seem to generate these, I guess that is what I get for using the one that comes from using the one in the package manager rather than building an up to date one. I will have a look at these extra warnings as well. Thanks, Charles