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

[Henrik Grimler]

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@xxxxxxxxxx>
> ---
> 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