Hi,

On Sun, Oct 31, 2021 at 09:35:20PM +0100, Krzysztof Kozlowski wrote:

On 31/10/2021 17:56, Henrik Grimler wrote:

quoted

Older Exynos socs has one reg PRO_ID containing both product id and
revision information. Newer Exynos socs has one Product_ID reg with
product id, and one CHIPID_REV reg with revision information.

In commit c072c4ef7ef0 ("soc: samsung: exynos-chipid: Pass revision
reg offsets") the driver was changed so that the revision part of
PRO_ID is masked to 0 when printed during probing. This can give a
false impression that the revision is 0, so lets change so entire
PRO_ID reg is printed again.

Signed-off-by: Henrik Grimler <redacted>
---
Has been tested on exynos4412-i9300, which is compatible with
exynos4210-chipid, and on an exynos8895 device compatible with
exynos850-chipid.
---

Hi,

Thanks for the patch.

I miss here however the most important information - why do you need it?
The answer to "why" should be in commit msg.

In dmesg we currently print something like:

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

where PRO_ID is given in datasheet as:

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

By printing PRO_ID[0xe4412000] it gives the impression that Package
information, Main Revision Number and Sub Revision Number are all 0.

The change was kind of intentional and accepted, because revision ID is
printed next to the product ID. Printing revision ID with product ID
could be confusing...

Sure, I see the reason for only printing the product id. Would you
accept a patch write Product_ID instead of PRO_ID in the printed
message? So that we print:

    Exynos: CPU[EXYNOS4412] Product_ID[0xe4412000] REV[0x11] Detected

There's then less room for confusion regarding the revision, since
Product_ID should contain only the Product ID, unlike PRO_ID which
should contain both Product ID and revision info.

Best regards,
Krzysztof

Best regards,
Henrik Grimler

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel