Re: [PATCH v3 1/3] soc: samsung: exynos-chipid: Pass revision reg offsets

Hi Sam,

On Thu, Oct 14, 2021 at 04:35:06PM +0300, Sam Protsenko wrote:

Old Exynos SoCs have both Product ID and Revision ID in one single
register, while new SoCs tend to have two separate registers for those
IDs. Implement handling of both cases by passing Revision ID register
offsets in driver data.

Previously existing macros for Exynos4210 (removed in this patch) were
incorrect:

    #define EXYNOS_SUBREV_MASK         (0xf << 4)
    #define EXYNOS_MAINREV_MASK        (0xf << 0)

Actual format of PRO_ID register in Exynos4210 (offset 0x0):

    [31:12] Product ID
      [9:8] Package information
      [7:4] Main Revision Number
      [3:0] Sub Revision Number

This patch doesn't change the behavior on existing platforms, so
'/sys/devices/soc0/revision' will show the same string as before.

Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
---
Changes in v2:
  - Renamed *_rev_bit fields in 'struct exynos_chipid_variant' to
    *_rev_shift
  - Renamed EXYNOS_REV_PART_LEN to EXYNOS_REV_PART_MASK
  - Renamed EXYNOS_REV_PART_OFF to EXYNOS_REV_PART_SHIFT

Changes in v3:
  - Rebased on top of krzk/for-next tree
  - Fixed wrong *_rev_shift values in exynos4210_chipid_drv_data
  - Implemented reading the register only once in case when both
    Product ID and Revision ID are stored in the same register
  - Tested all possible cases by emulating read register values
  - Provided more detailed explanation in commit message

 drivers/soc/samsung/exynos-chipid.c       | 69 +++++++++++++++++++----
 include/linux/soc/samsung/exynos-chipid.h |  6 +-
 2 files changed, 60 insertions(+), 15 deletions(-)

I tested this on exynos4412-i9300 and an exynos5420 tablet that is not
yet in mainline.  /sys/devices/soc0/* looks identical, I have:

$ cat /sys/devices/soc0/{family,revision,soc_id}
Samsung Exynos
11
EXYNOS4412

and

$ cat /sys/devices/soc0/{family,revision,soc_id}
Samsung Exynos
20
EXYNOS5420

before and after these patches.  The printed PRO_ID in dmesg changed
though, before I had:

[    0.894683] soc soc0: Exynos: CPU[EXYNOS4412] PRO_ID[0xe4412211] REV[0x11] Detected

[    4.964215] soc soc0: Exynos: CPU[EXYNOS5420] PRO_ID[0xe5420020] REV[0x20] Detected

But after it looks like package information and revision is missing
from the reported PRO_ID:

[    0.885515] soc soc0: Exynos: CPU[EXYNOS4412] PRO_ID[0xe4412000] REV[0x11] Detected

[    4.965560] soc soc0: Exynos: CPU[EXYNOS5420] PRO_ID[0xe5420000] REV[0x20] Detected

Best regards,
Henrik Grimler