mbox series

[v6,0/3] dt-bindings: arm: qcom: qcom,board-id and qcom,msm-id

Message ID 20220705130300.100882-1-krzysztof.kozlowski@linaro.org
Headers show
Series dt-bindings: arm: qcom: qcom,board-id and qcom,msm-id | expand

Message

Krzysztof Kozlowski July 5, 2022, 1:02 p.m. UTC 
Hi,

Changes since v5
================
1. Dual-license qcom,ids.h (Rob).
2. Minor corrections in comments.

Changes since v4
================
1. Change the qcom,board-id oneOf (oneOf at higher level) so newer dtschema is happy.

Changes since v3
================
1. Patch #1: Define all SoC IDs, based on Qualcomm socid driver (Konrad). Keep
   Dmitry Rb tag, even though it is quite a change.
2. New patch #2: use bindings in the socid driver.  The patch fails on checkpatch:
   "Macros with complex values should be enclosed in parentheses"
   but that's expected considering the macro contents.

Changes since v2
================
1. Adjust description of new fields after review (Dmitry).
2. Change name of msm8996 define (Dmitry).
3. Add Rb tags.

Changes since v1
================
1. Make the qcom,board-id and qcom,msm-id properties deprecated and limited to
   certain SoCs (Rob).
2. Extend the qcom,board-id schema to match OnePlus variant - four elements -
   and drop DTS patches splitting four into two touples (Stephan).

Description
===========
The discussion [1] brought several arguments for keeping the qcom,board-id and
qcom,msm-id properties.  Keeping means we should document them, so the DT
schema checks pass.

I revived old patch [2] with several changes and improvements.  The commit msg
hopefully collects feedback from the discussion.

Best regards,
Krzysztof

[1] https://lore.kernel.org/r/a3c932d1-a102-ce18-deea-18cbbd05ecab@linaro.org/
[2] https://lore.kernel.org/all/1425503602-24916-1-git-send-email-galak@codeaurora.org/

Krzysztof Kozlowski (3):
  dt-bindings: arm: qcom: document qcom,msm-id and qcom,board-id
  soc: qcom: socinfo: create soc_id table from bindings
  arm64: dts: qcom: msm8992-xiaomi-libra: split qcom,msm-id into tuples

 .../devicetree/bindings/arm/qcom.yaml         | 120 ++++++++
 .../boot/dts/qcom/msm8992-xiaomi-libra.dts    |   2 +-
 drivers/soc/qcom/socinfo.c                    | 259 +++++++++---------
 include/dt-bindings/arm/qcom,ids.h            | 152 ++++++++++
 4 files changed, 406 insertions(+), 127 deletions(-)
 create mode 100644 include/dt-bindings/arm/qcom,ids.h

Comments

Bjorn Andersson Aug. 29, 2022, 8:49 p.m. UTC | #1 
On Tue, Jul 05, 2022 at 03:02:59PM +0200, Krzysztof Kozlowski wrote:
> The Qualcomm SoC ID values are encoded in few places: DTS files,
> Devicetree bindings (both used by some of Qualcomm bootloaders or tools)
> and in soc_id table of socinfo driver.  Do not duplicate the actual
> values in the last one but use the constants from the bindings.
> 
> Tested by comparing output object file (exactly the same).
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

I didn't have a strong opinion either way on this, so was hoping someone
else would chime in. Doesn't seem like that has happened, but
unfortunately the soc_id[] list has changed.

Would you mind rebasing the two patches to match the latest list?

> ---
>  drivers/soc/qcom/socinfo.c | 259 +++++++++++++++++++------------------
>  1 file changed, 133 insertions(+), 126 deletions(-)
> 
> diff --git a/drivers/soc/qcom/socinfo.c b/drivers/soc/qcom/socinfo.c
> index cee579a267a6..d515d3a97f0e 100644
> --- a/drivers/soc/qcom/socinfo.c
> +++ b/drivers/soc/qcom/socinfo.c
> @@ -12,11 +12,14 @@
>  #include <linux/slab.h>
>  #include <linux/soc/qcom/smem.h>
>  #include <linux/string.h>
> +#include <linux/stringify.h>
>  #include <linux/sys_soc.h>
>  #include <linux/types.h>
>  
>  #include <asm/unaligned.h>
>  
> +#include <dt-bindings/arm/qcom,ids.h>
> +
>  /*
>   * SoC version type with major number in the upper 16 bits and minor
>   * number in the lower 16 bits.
> @@ -25,6 +28,10 @@
>  #define SOCINFO_MINOR(ver) ((ver) & 0xffff)
>  #define SOCINFO_VERSION(maj, min)  ((((maj) & 0xffff) << 16)|((min) & 0xffff))
>  
> +/* Helper macros to create soc_id table */
> +#define qcom_board_id(id) QCOM_ID_ ## id, __stringify(id)
> +#define qcom_board_id2(id, name) QCOM_ID_ ## id, (name)

How about naming this qcom_board_id_named() ?

Regards,
Bjorn
Bjorn Andersson Aug. 29, 2022, 11:45 p.m. UTC | #2 
On Tue, 5 Jul 2022 15:02:57 +0200, Krzysztof Kozlowski wrote:
> Changes since v5
> ================
> 1. Dual-license qcom,ids.h (Rob).
> 2. Minor corrections in comments.
> 
> Changes since v4
> ================
> 1. Change the qcom,board-id oneOf (oneOf at higher level) so newer dtschema is happy.
> 
> [...]

Applied, thanks!

[3/3] arm64: dts: qcom: msm8992-xiaomi-libra: split qcom,msm-id into tuples
      commit: 2b96ef794caa539d52f8e8c85ef907b3bc601c27

Best regards,
Krzysztof Kozlowski Aug. 30, 2022, 6:13 a.m. UTC | #3 
On 29/08/2022 23:49, Bjorn Andersson wrote:
> On Tue, Jul 05, 2022 at 03:02:59PM +0200, Krzysztof Kozlowski wrote:
>> The Qualcomm SoC ID values are encoded in few places: DTS files,
>> Devicetree bindings (both used by some of Qualcomm bootloaders or tools)
>> and in soc_id table of socinfo driver.  Do not duplicate the actual
>> values in the last one but use the constants from the bindings.
>>
>> Tested by comparing output object file (exactly the same).
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> I didn't have a strong opinion either way on this, so was hoping someone
> else would chime in. Doesn't seem like that has happened, but
> unfortunately the soc_id[] list has changed.
> 
> Would you mind rebasing the two patches to match the latest list?

Sure.

> 
>> ---
>>  drivers/soc/qcom/socinfo.c | 259 +++++++++++++++++++------------------
>>  1 file changed, 133 insertions(+), 126 deletions(-)
>>
>> diff --git a/drivers/soc/qcom/socinfo.c b/drivers/soc/qcom/socinfo.c
>> index cee579a267a6..d515d3a97f0e 100644
>> --- a/drivers/soc/qcom/socinfo.c
>> +++ b/drivers/soc/qcom/socinfo.c
>> @@ -12,11 +12,14 @@
>>  #include <linux/slab.h>
>>  #include <linux/soc/qcom/smem.h>
>>  #include <linux/string.h>
>> +#include <linux/stringify.h>
>>  #include <linux/sys_soc.h>
>>  #include <linux/types.h>
>>  
>>  #include <asm/unaligned.h>
>>  
>> +#include <dt-bindings/arm/qcom,ids.h>
>> +
>>  /*
>>   * SoC version type with major number in the upper 16 bits and minor
>>   * number in the lower 16 bits.
>> @@ -25,6 +28,10 @@
>>  #define SOCINFO_MINOR(ver) ((ver) & 0xffff)
>>  #define SOCINFO_VERSION(maj, min)  ((((maj) & 0xffff) << 16)|((min) & 0xffff))
>>  
>> +/* Helper macros to create soc_id table */
>> +#define qcom_board_id(id) QCOM_ID_ ## id, __stringify(id)
>> +#define qcom_board_id2(id, name) QCOM_ID_ ## id, (name)
> 
> How about naming this qcom_board_id_named() ?


Yes, sure.


Best regards,
Krzysztof