[v6,00/14] Add support for magnetometer Yamaha YAS537

Message

Jakob Hauser Aug. 12, 2022, 9:54 p.m. UTC

This patchset adds YAS537 variant to the already existing driver for
Yamaha YAS magnetometers.

Patch 1 is a fix on the current driver.
Patches 2-13 are cleanups and refactoring.
Patch 14 finally adds the YAS537 variant.

Changes in v6:
 - Rebased to torvalds/linux master (shortly before releasing v5.20-rc1)
   to include Jonathan's patch "iio: magn: yas530: Use
   DEFINE_RUNTIME_DEV_PM_OPS() and pm_ptr() macros"
 - Patch 6: In commit message corrected "This is a preparation for adding
   the YAS537 variant."
 - Patch 6: Added period at the end of multi-line comments (3x "Used by
   YAS530, YAS532 and YAS533.")
 - Patch 7: In commit message replaced "Additionally" by "While at it".
 - Patch 9: Moved content of "product_name" and "version_names" from array
   directly into chip_info table due to C11 standard 6.6.7 considering array
   calls not as constant expression.
 - Patch 9: In commit message replaced "This commit introduces" by "Introduce".
 - Patch 9 & following: Introduced variable "ci" to substitute
   "yas5xx->chip_info".
 - Patch 9: Removed introduction of "enum chip_ids" as "yas5xx->chip". Instead,
   in the function yas5xx_probe(), used "id->driver_data" directly to
   initialize enumeration of "yas5xx_chip_info_tbl[]".
 - Patch 12: Moved values "t_ref" and "min_temp_x10" from array directly into
   chip_info table due to C11 standard 6.6.7 considering array calls not as
   constant expression. Comments were partially moved into the kernel doc of
   struct yas5xx_chip_info.
 - Patch 12: In the function yas5xx_calc_temperature(), changed data type of
   min_temp_x10 from int to s16 (as it's declared in struct yas5xx_chip_info).
 - Patch 13: In commit message corrected typo in word "function".
 - Patch 14: In commit message changed "This adds support" to "Add support".
 - Patch 14: In commit message changed wording "for YAS537 in the mainline".

Changes in v5:
 - Rebased to torvalds/linux v5.19.
 - Patch 6: Moved 3x comment "Used by YAS530, YAS532 and YAS533" into
   kernel doc.
 - Patch 6: Corrected missing renaming of 2x "yas530_get_measure",
   1x "yas530_power_on", 1x "YAS530_ACTUATE_INIT_COIL", 1x "YAS530_MEASURE".
 - Patch 7: Added "Suggested-by:" tag.
 - Split patch 9 v4 "Introduce 'chip_info' structure" into five patches 9-13.
 - Patch 9: In commit message changed wording "Device Tree".
 - Patch 9: Added commas to non-terminating arrays of "yas5xx_product_name",
   "yas5xx_version_name", and "yas5xx_chip_info".
 - Patch 9: Added "product_name" and "version_name" to "chip_info" struct.
 - Patch 9: Added "s" to array name "yas5xx_version_names".
 - Patch 9: Added indices to array "yas5xx_version_names".
 - Patch 9: For strings in arrays "yas5xx_product_name" and
   "yas5xx_version_names", applied char * instead of char [].
 - Patch 9: Initialize struct "chip_info" as const.
 - Patch 9: In function yas5xx_probe(), moved declaration of "id_check"
   to a new line.
 - Patch 9: In function yas5xx_probe(), after "if (id_check !=
   yas5xx->chip_info->devid)" applied dev_err_probe().
 - Patch 10: In function yas5xx_volatile_reg(), renamed integer "j" into
   "reg_qty".
 - Patch 12: Improved comments on arrays "t_ref_counts" and
   "min_temp_celsius_x10".
 - Patch 12: Changed arrays "t_ref_counts" and "min_temp_celsius_x10"
   to static.
 - Patch 12: Corrected wrong spelling of "celcius" with "c" in array
   "min_temp_celsius_x10"
 - Patch 13: In function yas5xx_probe(), added the conditional
   "if (yas5xx->chip_info->measure_offsets)" as a preparatory step for YAS537.
 - Patch 14: In function yas537_power_on(), replaced comment "Write registers
   according to Android driver" by "Writing ADCCAL and TRM registers".
 - Patch 14: In function yas537_power_on(), write register ADCCAL as a
   bulk write.
 - Patch 14: In function yas537_power_on(), in formula for "intrvl" replaced
   value "1000" by MILLI. Added "linux/units.h" to includes.

Changes in v4:
 - Rebased to torvalds/linux v5.19-rc4, as this now includes Linus' patch
   "Fix memchr_inv() misuse" on driver yamaha-yas530.
 - Removed redundant Cc tags.
 - Patch 2: Replaced "<= ... + 7" by "< ... + 8" and adapted commit message.
 - Patch 3: Added default for switch statement, I forgot to add this.
 - Patch 4: In function yas5xx_get_measure(), changed wording "milli degrees"
   to "millidegrees".
 - Patch 6: Changed the renaming of function from "yas530_532_" to "yas530_".
   Same for registers. Added additional comments where appropriate.
 - Patch 6: Removed "Reviewed-by:" tag of Andy.
 - Split patch 7 v3 into two patches -> patch 7 v4 and patch 8 v4.
 - Patch 8: Applied "if (a && b)" suggestion at memchr_inv() by Andy in
   function yas532_get_calibration_data().
 - Patch 8: Removed defines for device IDs YAS537 and YAS539 and accordingly
   the comment "These variant IDs are known from code dumps".
 - Patch 9: New patch to introduce the "chip_info" approach.
 - Patch 10: In function yas537_get_calibration_data(), removed "the exact
   meaning is unknown" from comment "Writing SRST register".
 - Patch 10: Also applied "if (a && b)" suggestion at memchr_inv() by Andy
   in function yas537_get_calibration_data(). Additionally changed the second
   condition from "== 0" to "!".
 - Patch 10: In function yas537_get_calibration_data(), removed empty lines
   within switch statement. In that context, removed comment "Get data into
   these four blocks val1 to val4".
 - Patch 10: In Kconfig, simplified wording.
 - Patch 10: In function yas537_get_calibration_data() "case YAS537_VERSION_0",
   reduced indent of "for" loop by splitting it into multiple loops. I didn't
   use integer j, as it was suggested by Jonathan, because only using integer i
   is more consistant with the loop in "case YAS537_VERSION_1".
 - Accordingly, split the "for" loop in "case YAS537_VERSION_1" into two loops
   as well. Technically this isn't neccessary but it improves readability.
 - Patch 10: Added new defines of masks for YAS537 and applied these by
   FIELD_PREP and FIELD_GET in function yas537_get_calibration_data()
   within "case YAS537_VERSION_1".
 - Patch 10: In function yas537_power_on(), added spaced at "YAS537_ADCCAL + 1".

Changes in v3:
 - In patch 3 fixed 2x typo "Divide".
 - In commit message of patch 4 fixed wording "in the yas5xx_get_measure()
   function".
 - In patch 4 in the comment for the temperature calculation fixed wording
   "is the number of counts".
 - In patch 4 added defaults to switch statements.
 - Splitted stray changes into new patch 7 v3. "Add YAS537 variant" is now
   patch 8 v3. I haven't added "Reviewed-by:" tag of Linus to patch 7 v3
   because as a separate patch these changes appear in a different context.
 - In new patch 7 v3, changed printk format specifiers in the function
   yas530_get_calibration_data() to "%16ph" and in the function
   yas532_get_calibration_data() to "%14ph". The first one is also a minor
   correction in behaviour, as the calibration data array size of YAS530
   is 16 (the dev_dbg printed 14 before).
 - Rebased to linux-next to include patch bb52d3691db8 "iio: magnetometer:
   yas530: Fix memchr_inv() misuse".
 - In patch 7 v3, changed memchr_inv() line for YAS532.
 - In patch 8 v3 in the function yas537_get_calibration_data(), changed
   memchr_inv() line for YAS537.
 - Removed comment "corresponds to 0x70" at define YAS537_MAG_AVERAGE_32_MASK.
 - Added suffixes _US and _MS in defines for YAS537.
 - In the function yas537_measure(), removed comments "Read data", "Arrange
   data", "Assign data".
 - In the function yas537_measure(), replaced bitwise shift by
   get_unaligned_be16().
 - Replaced "if (h[i] < -8192)" etc. by clamp_val().
 - In the functions yas537_measure() and yas537_get_measure(), replaced 8192
   by BIT(13) and 16384 by BIT(14).
 - Fixed typo "resolution" in the function yas5xx_read_raw().
 - Fixed typo "Divide" in patch 8 v3 in the function yas5xx_read_raw().
 - In patch 8 v3 in the yas537_get_calibration_data(), changed printk format
   specifier to "%17ph"
 - In the functions yas537_measure() and yas537_get_calibration_data(), drop
   some parentheses in regmap_write().
 - In the function yas537_power_on(), added comment "Wait until the coil has
   ramped up".
 - In the function yas5xx_probe(), put YAS537 variant and version printings
   into one print.
 - In the function yas537_get_measure(), fixed wording "is the number of
   counts" in the comment for the temperature calculation.
 - In the function yas537_get_measure(), added product description document No.
   into the comment for the temperature calculation (as I first thought the
   review comment "the number" is related to this).
 - In the function yas537_get_calibration_data(), corrected comment "Get data
   into these four blocks val1 to val4".

Changes in v2:
 - Reordered the patchset by moving patch 4 v1 to patch 1 v2.
 - Removed patch 6 v1 ("Remove redundant defaults on switch devid")
 - Accordingly, added "default:" to each switch statement in patch 7.
 - Moved renamings in patch 7 v1 into a separate new patch 6 v2. I added
   the "Reviewed-by:" tag of Linus to both patches, hope that's ok, else
   feel free to comment.
 - Removed regmap reads and related debug dumps in patch 7 in function
   yas537_dump_calibration(). As this function now applies to version 1
   only, replaced switch statement by if clause.
 - Also removed "hard_offsets" debug dumps in that function.
 - Fixed typo "initialized" in commit message of patch 7.

v5: https://lore.kernel.org/linux-iio/[e-mail]/T/#t
v4: https://lore.kernel.org/linux-iio/[e-mail]/T/#t
v3: https://lore.kernel.org/linux-iio/[e-mail]/T/#t
v2: https://lore.kernel.org/linux-iio/[e-mail]/T/#t
v1: https://lore.kernel.org/linux-iio/[e-mail]/T/#t

Jakob Hauser (14):
  iio: magnetometer: yas530: Change data type of hard_offsets to signed
  iio: magnetometer: yas530: Change range of data in volatile register
  iio: magnetometer: yas530: Correct scaling of magnetic axes
  iio: magnetometer: yas530: Correct temperature handling
  iio: magnetometer: yas530: Change data type of calibration
    coefficients
  iio: magnetometer: yas530: Rename functions and registers
  iio: magnetometer: yas530: Move printk %*ph parameters out from stack
  iio: magnetometer: yas530: Apply documentation and style fixes
  iio: magnetometer: yas530: Introduce "chip_info" structure
  iio: magnetometer: yas530: Add volatile registers to "chip_info"
  iio: magnetometer: yas530: Add IIO scaling to "chip_info"
  iio: magnetometer: yas530: Add temperature calculation to "chip_info"
  iio: magnetometer: yas530: Add function pointers to "chip_info"
  iio: magnetometer: yas530: Add YAS537 variant

 drivers/iio/magnetometer/Kconfig         |   4 +-
 drivers/iio/magnetometer/yamaha-yas530.c | 830 +++++++++++++++++++----
 2 files changed, 703 insertions(+), 131 deletions(-)

Comments

Jonathan Cameron Aug. 14, 2022, 4:54 p.m. UTC | #1

On Fri, 12 Aug 2022 23:54:05 +0200
Jakob Hauser <[e-mail]> wrote:

> This patchset adds YAS537 variant to the already existing driver for
> Yamaha YAS magnetometers.
> 
> Patch 1 is a fix on the current driver.
> Patches 2-13 are cleanups and refactoring.
> Patch 14 finally adds the YAS537 variant.

Series applied.  That build bot issue on v5 took me rather by surprise.
Good to see the resolution looks pretty nice.

I've applied this to what will become the togreg branch after rc1 is out
and I can rebase. In the meantime pushed out as testing so the build bots
are forced to take another look at it on top of my current tree.

Thanks to all who have been reviewing this one and to Jakob for persisting.
Unless anyone spots anything critical, let us do as Andy suggested and
carry out any further tweaks as follow on patches.

Thanks,

Jonathan