* [PATCH 1/2] soc: samsung: exynos-chipid: print entire PRO_ID reg when probing
@ 2021-10-31 16:56 Henrik Grimler
2021-10-31 16:56 ` [PATCH 2/2] soc: samsung: exynos-chipid: be more informative when soc is unknown Henrik Grimler
2021-10-31 20:35 ` [PATCH 1/2] soc: samsung: exynos-chipid: print entire PRO_ID reg when probing Krzysztof Kozlowski
0 siblings, 2 replies; 6+ messages in thread
From: Henrik Grimler @ 2021-10-31 16:56 UTC (permalink / raw)
To: krzysztof.kozlowski, chanho61.park, semen.protsenko
Cc: linux-arm-kernel, linux-samsung-soc, Henrik Grimler
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 <henrik@grimler.se>
---
Has been tested on exynos4412-i9300, which is compatible with
exynos4210-chipid, and on an exynos8895 device compatible with
exynos850-chipid.
---
drivers/soc/samsung/exynos-chipid.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
index a28053ec7e6a..7fe44f71920d 100644
--- a/drivers/soc/samsung/exynos-chipid.c
+++ b/drivers/soc/samsung/exynos-chipid.c
@@ -33,6 +33,7 @@ struct exynos_chipid_variant {
};
struct exynos_chipid_info {
+ u32 pro_id;
u32 product_id;
u32 revision;
};
@@ -79,6 +80,7 @@ static int exynos_chipid_get_chipid_info(struct regmap *regmap,
ret = regmap_read(regmap, EXYNOS_CHIPID_REG_PRO_ID, &val);
if (ret < 0)
return ret;
+ soc_info->pro_id = val;
soc_info->product_id = val & EXYNOS_MASK;
if (data->rev_reg != EXYNOS_CHIPID_REG_PRO_ID) {
@@ -146,7 +148,7 @@ static int exynos_chipid_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, soc_dev);
dev_info(&pdev->dev, "Exynos: CPU[%s] PRO_ID[0x%x] REV[0x%x] Detected\n",
- soc_dev_attr->soc_id, soc_info.product_id, soc_info.revision);
+ soc_dev_attr->soc_id, soc_info.pro_id, soc_info.revision);
return 0;
base-commit: b417d1e88f32645ed62a00d43c347b4386a0a021
--
2.33.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] soc: samsung: exynos-chipid: be more informative when soc is unknown
2021-10-31 16:56 [PATCH 1/2] soc: samsung: exynos-chipid: print entire PRO_ID reg when probing Henrik Grimler
@ 2021-10-31 16:56 ` Henrik Grimler
2021-10-31 20:35 ` Krzysztof Kozlowski
2021-10-31 20:35 ` [PATCH 1/2] soc: samsung: exynos-chipid: print entire PRO_ID reg when probing Krzysztof Kozlowski
1 sibling, 1 reply; 6+ messages in thread
From: Henrik Grimler @ 2021-10-31 16:56 UTC (permalink / raw)
To: krzysztof.kozlowski, chanho61.park, semen.protsenko
Cc: linux-arm-kernel, linux-samsung-soc, Henrik Grimler
Print the obtained product id that is incompatible.
Signed-off-by: Henrik Grimler <henrik@grimler.se>
---
drivers/soc/samsung/exynos-chipid.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
index 7fe44f71920d..885d578fd005 100644
--- a/drivers/soc/samsung/exynos-chipid.c
+++ b/drivers/soc/samsung/exynos-chipid.c
@@ -132,7 +132,7 @@ static int exynos_chipid_probe(struct platform_device *pdev)
"%x", soc_info.revision);
soc_dev_attr->soc_id = product_id_to_soc_id(soc_info.product_id);
if (!soc_dev_attr->soc_id) {
- pr_err("Unknown SoC\n");
+ pr_err("Exynos: Unknown SoC: 0x%x\n", soc_info.product_id);
return -ENODEV;
}
--
2.33.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] soc: samsung: exynos-chipid: print entire PRO_ID reg when probing
2021-10-31 16:56 [PATCH 1/2] soc: samsung: exynos-chipid: print entire PRO_ID reg when probing Henrik Grimler
2021-10-31 16:56 ` [PATCH 2/2] soc: samsung: exynos-chipid: be more informative when soc is unknown Henrik Grimler
@ 2021-10-31 20:35 ` Krzysztof Kozlowski
2021-10-31 21:29 ` Henrik Grimler
1 sibling, 1 reply; 6+ messages in thread
From: Krzysztof Kozlowski @ 2021-10-31 20:35 UTC (permalink / raw)
To: Henrik Grimler, chanho61.park, semen.protsenko
Cc: linux-arm-kernel, linux-samsung-soc
On 31/10/2021 17:56, Henrik Grimler wrote:
> 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 <henrik@grimler.se>
> ---
> 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.
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...
Best regards,
Krzysztof
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] soc: samsung: exynos-chipid: print entire PRO_ID reg when probing
2021-10-31 20:35 ` [PATCH 1/2] soc: samsung: exynos-chipid: print entire PRO_ID reg when probing Krzysztof Kozlowski
@ 2021-10-31 21:29 ` Henrik Grimler
2021-11-01 7:43 ` Krzysztof Kozlowski
0 siblings, 1 reply; 6+ messages in thread
From: Henrik Grimler @ 2021-10-31 21:29 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: chanho61.park, semen.protsenko, linux-arm-kernel,
linux-samsung-soc
Hi,
On Sun, Oct 31, 2021 at 09:35:20PM +0100, Krzysztof Kozlowski wrote:
> On 31/10/2021 17:56, Henrik Grimler wrote:
> > 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 <henrik@grimler.se>
> > ---
> > 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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] soc: samsung: exynos-chipid: print entire PRO_ID reg when probing
2021-10-31 21:29 ` Henrik Grimler
@ 2021-11-01 7:43 ` Krzysztof Kozlowski
0 siblings, 0 replies; 6+ messages in thread
From: Krzysztof Kozlowski @ 2021-11-01 7:43 UTC (permalink / raw)
To: Henrik Grimler
Cc: chanho61.park, semen.protsenko, linux-arm-kernel,
linux-samsung-soc
On 31/10/2021 22:29, Henrik Grimler wrote:
> Hi,
>
> On Sun, Oct 31, 2021 at 09:35:20PM +0100, Krzysztof Kozlowski wrote:
>> On 31/10/2021 17:56, Henrik Grimler wrote:
>>> 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 <henrik@grimler.se>
>>> ---
>>> 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.
Yes, let it be then:
Exynos: CPU[EXYNOS4412] ProductID[0xe4412000] Rev[0x11] detected
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-11-01 7:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-10-31 16:56 [PATCH 1/2] soc: samsung: exynos-chipid: print entire PRO_ID reg when probing Henrik Grimler
2021-10-31 16:56 ` [PATCH 2/2] soc: samsung: exynos-chipid: be more informative when soc is unknown Henrik Grimler
2021-10-31 20:35 ` Krzysztof Kozlowski
2021-10-31 20:35 ` [PATCH 1/2] soc: samsung: exynos-chipid: print entire PRO_ID reg when probing Krzysztof Kozlowski
2021-10-31 21:29 ` Henrik Grimler
2021-11-01 7:43 ` Krzysztof Kozlowski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox