Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 14 Oct 2021 20:40:05 +0200
From:   Henrik Grimler <henrik@grimler.se>
To:     Sam Protsenko <semen.protsenko@linaro.org>
Cc:     Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>,
        Rob Herring <robh+dt@kernel.org>,
        Sumit Semwal <sumit.semwal@linaro.org>,
        linux-samsung-soc@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 1/3] soc: samsung: exynos-chipid: Pass revision reg
 offsets
Message-ID: <YWh5hSvUpdFA85Ct@grimlerstat.localdomain>
References: <20211014133508.1210-1-semen.protsenko@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20211014133508.1210-1-semen.protsenko@linaro.org>
Precedence: bulk

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