Skip to contentSkip to conversation

dt-binding: ipmi: add fallback to npcm845 compatible

PATCH Superseded Tomer Maimon 3 participants 14 messages Aug 4, 2022 – Aug 8, 2022

Subsystems (3)
Device tree bindings IPMI Arm/nuvoton npcm architecture

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Tomer Maimon

author maintainer
#0 lore raw

PATCH v2 1/1 dt-binding: ipmi: add fallback to npcm845 compatible

Add to npcm845 KCS compatible string a fallback to npcm750 KCS compatible string becuase NPCM845 and NPCM750 BMCs are using identical KCS modules.

Signed-off-by Tomer Maimon <[email protected]>
---
 Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt +1 -1
index cbc10a68ddef..4fda76e63396 100644
@@ -7,7 +7,7 @@ used to perform in-band IPMI communication with their host.
 Required properties:
 - compatible : should be one of
     "nuvoton,npcm750-kcs-bmc"
-    "nuvoton,npcm845-kcs-bmc"
+    "nuvoton,npcm845-kcs-bmc", "nuvoton,npcm750-kcs-bmc"
 - interrupts : interrupt generated by the controller
 - kcs_chan : The KCS channel number in the controller
Signature · 1 line
2.33.0
#1 lore raw
Quoted 4 lines · Tomer Maimon

Add to npcm845 KCS compatible string a fallback to npcm750 KCS compatible string becuase NPCM845 and NPCM750 BMCs are using identical KCS modules.

Signed-off-by Tomer Maimon <[email protected]>

Your previous commit adding that compatible was simply wrong and not matching the driver and it is not the first time. I think all Nuvoton patches need much more careful review :(

You forgot the fixes tag:

Fixes 84261749e58a ("dt-bindings: ipmi: Add npcm845 compatible")

Best regards,
Krzysztof

author maintainer
#2 lore raw

Hi Krzysztof,

Thanks for your review.

Quoted 10 lines · Krzysztof Kozlowski
Quoted 4 lines · Tomer Maimon

Add to npcm845 KCS compatible string a fallback to npcm750 KCS compatible string becuase NPCM845 and NPCM750 BMCs are using identical KCS modules.

Signed-off-by Tomer Maimon <[email protected]>

Your previous commit adding that compatible was simply wrong and not matching the driver and it is not the first time. I think all Nuvoton patches need much more careful review :(

Will do and sorry about all the mess...

Quoted 4 lines

You forgot the fixes tag:

Fixes 84261749e58a ("dt-bindings: ipmi: Add npcm845 compatible")

Will add the tag next version.

Quoted 6 lines

Best regards,
Krzysztof

Best regards,

Tomer

#3 lore raw
Quoted 16 lines · Tomer Maimon

Hi Krzysztof,

Thanks for your review.

Quoted 10 lines · Krzysztof Kozlowski
Quoted 4 lines · Tomer Maimon

Add to npcm845 KCS compatible string a fallback to npcm750 KCS compatible string becuase NPCM845 and NPCM750 BMCs are using identical KCS modules.

Signed-off-by Tomer Maimon <[email protected]>

Your previous commit adding that compatible was simply wrong and not matching the driver and it is not the first time. I think all Nuvoton patches need much more careful review :(

Quoted 6 lines

Will do and sorry about all the mess...

Quoted 4 lines

You forgot the fixes tag:

Fixes 84261749e58a ("dt-bindings: ipmi: Add npcm845 compatible")

Will add the tag next version.

You received a bit different review from Corey, so to be clear:
1. Your approach is correct, assuming the devices are really compatible.
2. Add a fixes tag and send a v3, to get my ack.

Best regards,
Krzysztof

author maintainer
#4 lore raw

Hi Krzysztof,

Thanks for your reply.

Quoted 20 lines · Krzysztof Kozlowski
Quoted 18 lines · Tomer Maimon

Hi Krzysztof,

Thanks for your review.

Quoted 10 lines · Krzysztof Kozlowski
Quoted 4 lines · Tomer Maimon

Add to npcm845 KCS compatible string a fallback to npcm750 KCS compatible string becuase NPCM845 and NPCM750 BMCs are using identical KCS modules.

Signed-off-by Tomer Maimon <[email protected]>

Your previous commit adding that compatible was simply wrong and not matching the driver and it is not the first time. I think all Nuvoton patches need much more careful review :(

Will do and sorry about all the mess...

Quoted 1 line
Quoted 8 lines
Quoted 4 lines
Quoted 3 lines

You forgot the fixes tag:

Fixes 84261749e58a ("dt-bindings: ipmi: Add npcm845 compatible")

Will add the tag next version.

You received a bit different review from Corey, so to be clear:
1. Your approach is correct, assuming the devices are really compatible.
2. Add a fixes tag and send a v3, to get my ack.

Will do.

Quoted 3 lines

Best regards,
Krzysztof

Best regards,

Tomer

#5 lore raw
In reply to Tomer Maimon
Quoted 13 lines · Tomer Maimon

Add to npcm845 KCS compatible string a fallback to npcm750 KCS compatible string becuase NPCM845 and NPCM750 BMCs are using identical KCS modules.

Signed-off-by Tomer Maimon <[email protected]>
---
 Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt +0 -0
index cbc10a68ddef..4fda76e63396 100644

Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt used to perform in-band IPMI communication with their host. · line 10

Quoted 5 lines
Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt +1 -1
@@ -7,7 +7,7 @@ used to perform in-band IPMI communication with their host.
 Required properties:
 - compatible : should be one of
     "nuvoton,npcm750-kcs-bmc"
-    "nuvoton,npcm845-kcs-bmc"
+    "nuvoton,npcm845-kcs-bmc", "nuvoton,npcm750-kcs-bmc"

This is just wrong. The compatible is supposed to identify the device, not the board the device is on. I think compatible here should be "npcm7xx-kcs-bmc", and just use that everywhere. It's fine if that is used on a board named npcm845.

-corey

Quoted 6 lines
Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt +0 -0
 - interrupts : interrupt generated by the controller
 - kcs_chan : The KCS channel number in the controller
Signature · 1 line
2.33.0
author maintainer
#6 lore raw

Hi Corey,

Thanks for your comment.

Quoted 10 lines · Corey Minyard
Quoted 8 lines · Tomer Maimon

Add to npcm845 KCS compatible string a fallback to npcm750 KCS compatible string becuase NPCM845 and NPCM750 BMCs are using identical KCS modules.

Signed-off-by Tomer Maimon <[email protected]>
---
 Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 15 lines

Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt used to perform in-band IPMI communication with their host. · line 10

Quoted 10 lines
Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt +1 -1
index cbc10a68ddef..4fda76e63396 100644
@@ -7,7 +7,7 @@ used to perform in-band IPMI communication with their host.
 Required properties:
 - compatible : should be one of
     "nuvoton,npcm750-kcs-bmc"
-    "nuvoton,npcm845-kcs-bmc"
+    "nuvoton,npcm845-kcs-bmc", "nuvoton,npcm750-kcs-bmc"

This is just wrong. The compatible is supposed to identify the device, not the board the device is on. I think compatible here should be "npcm7xx-kcs-bmc", and just use that everywhere. It's fine if that is used on a board named npcm845.

The NPCM8XX is not a board, The Nuvoton NPCM8XX is a fourth-generation BMC SoC device family.

Quoted 9 lines

-corey

Quoted 6 lines
Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt +0 -1
 - interrupts : interrupt generated by the controller
 - kcs_chan : The KCS channel number in the controller
 
--

2.33.0

Best regards,

Tomer

#7 lore raw
Quoted 30 lines · Tomer Maimon

Hi Corey,

Thanks for your comment.

Quoted 10 lines · Corey Minyard
Quoted 8 lines · Tomer Maimon

Add to npcm845 KCS compatible string a fallback to npcm750 KCS compatible string becuase NPCM845 and NPCM750 BMCs are using identical KCS modules.

Signed-off-by Tomer Maimon <[email protected]>
---
 Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 15 lines

Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt used to perform in-band IPMI communication with their host. · line 10

Quoted 10 lines
Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt +1 -1
index cbc10a68ddef..4fda76e63396 100644
@@ -7,7 +7,7 @@ used to perform in-band IPMI communication with their host.
 Required properties:
 - compatible : should be one of
     "nuvoton,npcm750-kcs-bmc"
-    "nuvoton,npcm845-kcs-bmc"
+    "nuvoton,npcm845-kcs-bmc", "nuvoton,npcm750-kcs-bmc"

This is just wrong. The compatible is supposed to identify the device, not the board the device is on. I think compatible here should be "npcm7xx-kcs-bmc", and just use that everywhere. It's fine if that is used on a board named npcm845.

Quoted 2 lines

The NPCM8XX is not a board, The Nuvoton NPCM8XX is a fourth-generation BMC SoC device family.

Ok, but same principle applies.

If the device is exactly the same, then you would only use one of the "npcm7xx-kcs-bmc" and put that in both device trees. You can use "nuvoton,npcm750-kcs-bmc", it's really not that important. Or even "nuvoton,npcm-kcs-bmc"

If the device has a minor difference that can be expressed in a parameter, then create a parameter for it.

If the device has enough differences that a parameter or two doesn't cover it, then you put either nuvoton,npcm750-kcs-bmc or nuvoton,npcm750-kcs-bmc in the device tree. Not both. Then you need two entries in the of_device_id array and you use the data field or something to express the difference.

Since there appears to be no difference, just put "nuvoton,npcm750-kcs-bmc" in the npcm845 and I will drop the patch adding all this. Then a patch can be added saying it applies to both the 7xx and 8xx series of BMC SOCs. If you want to change the name, then a patch will be needed for that, but then you will need multiple entries in your device tree, but you would not document it as such, as there would only be one that applies for this kernel.

I'm pretty sure the only reason to have muliple compatible entries in a device tree is to cover multiple kernels where the name changed.

-corey

Quoted 13 lines
Quoted 9 lines

-corey

Quoted 6 lines
Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt +0 -1
 - interrupts : interrupt generated by the controller
 - kcs_chan : The KCS channel number in the controller
 
--

2.33.0

Best regards,

Tomer

author maintainer
#8 lore raw
Quoted 35 lines · Corey Minyard
Quoted 30 lines · Tomer Maimon

Hi Corey,

Thanks for your comment.

Quoted 10 lines · Corey Minyard
Quoted 8 lines · Tomer Maimon

Add to npcm845 KCS compatible string a fallback to npcm750 KCS compatible string becuase NPCM845 and NPCM750 BMCs are using identical KCS modules.

Signed-off-by Tomer Maimon <[email protected]>
---
 Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 15 lines

Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt used to perform in-band IPMI communication with their host. · line 10

Quoted 10 lines
Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt +1 -1
index cbc10a68ddef..4fda76e63396 100644
@@ -7,7 +7,7 @@ used to perform in-band IPMI communication with their host.
 Required properties:
 - compatible : should be one of
     "nuvoton,npcm750-kcs-bmc"
-    "nuvoton,npcm845-kcs-bmc"
+    "nuvoton,npcm845-kcs-bmc", "nuvoton,npcm750-kcs-bmc"

This is just wrong. The compatible is supposed to identify the device, not the board the device is on. I think compatible here should be "npcm7xx-kcs-bmc", and just use that everywhere. It's fine if that is used on a board named npcm845.

Quoted 2 lines

The NPCM8XX is not a board, The Nuvoton NPCM8XX is a fourth-generation BMC SoC device family.

Quoted 6 lines

Ok, but same principle applies.

If the device is exactly the same, then you would only use one of the "npcm7xx-kcs-bmc" and put that in both device trees. You can use "nuvoton,npcm750-kcs-bmc", it's really not that important. Or even "nuvoton,npcm-kcs-bmc"

If we use "nuvoton, npcm-kcs-bmc" we should take care of backward dts compatibility, and I am not sure we like to change NPCM KCS driver.

Quoted 10 lines

If the device has a minor difference that can be expressed in a parameter, then create a parameter for it.

If the device has enough differences that a parameter or two doesn't cover it, then you put either nuvoton,npcm750-kcs-bmc or nuvoton,npcm750-kcs-bmc in the device tree. Not both. Then you need two entries in the of_device_id array and you use the data field or something to express the difference.

Quoted 7 lines

Since there appears to be no difference, just put "nuvoton,npcm750-kcs-bmc" in the npcm845 and I will drop the patch adding all this. Then a patch can be added saying it applies to both the 7xx and 8xx series of BMC SOCs. If you want to change the name, then a patch will be needed for that, but then you will need multiple entries in your device tree, but you would not document it as such, as there would only be one that applies for this kernel.

It little bit confusing to use nuvoton,npcm750-kcs-bmc that are related to NPCM7XX for NPCM8XX KCS.
We can use the generic name "nuvoton, npcm-kcs-bmc" as you suggested above but we should take care of backward dts compatibility, and I am not sure we like to change NPCM KCS driver.

We had a disscation with Arnd, Arnd asked us to use a fallback as we did here if NPCM8XX device module is similar to NPCM7XX module:
https://lore.kernel.org/lkml/[email protected]/

I think we should use a fallback to describe the NPCM8XX KCS in the dt-binding document.

Quoted 19 lines

I'm pretty sure the only reason to have muliple compatible entries in a device tree is to cover multiple kernels where the name changed.

-corey

Quoted 13 lines
Quoted 9 lines

-corey

Quoted 6 lines
Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt +0 -1
 - interrupts : interrupt generated by the controller
 - kcs_chan : The KCS channel number in the controller
 
--

2.33.0

Best regards,

Tomer

Best regards,

Tomer

#9 lore raw
Quoted 62 lines · Tomer Maimon
Quoted 35 lines · Corey Minyard
Quoted 30 lines · Tomer Maimon

Hi Corey,

Thanks for your comment.

Quoted 10 lines · Corey Minyard
Quoted 8 lines · Tomer Maimon

Add to npcm845 KCS compatible string a fallback to npcm750 KCS compatible string becuase NPCM845 and NPCM750 BMCs are using identical KCS modules.

Signed-off-by Tomer Maimon <[email protected]>
---
 Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 15 lines

Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt used to perform in-band IPMI communication with their host. · line 10

Quoted 10 lines
Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt +1 -1
index cbc10a68ddef..4fda76e63396 100644
@@ -7,7 +7,7 @@ used to perform in-band IPMI communication with their host.
 Required properties:
 - compatible : should be one of
     "nuvoton,npcm750-kcs-bmc"
-    "nuvoton,npcm845-kcs-bmc"
+    "nuvoton,npcm845-kcs-bmc", "nuvoton,npcm750-kcs-bmc"

This is just wrong. The compatible is supposed to identify the device, not the board the device is on. I think compatible here should be "npcm7xx-kcs-bmc", and just use that everywhere. It's fine if that is used on a board named npcm845.

Quoted 2 lines

The NPCM8XX is not a board, The Nuvoton NPCM8XX is a fourth-generation BMC SoC device family.

Quoted 6 lines

Ok, but same principle applies.

If the device is exactly the same, then you would only use one of the "npcm7xx-kcs-bmc" and put that in both device trees. You can use "nuvoton,npcm750-kcs-bmc", it's really not that important. Or even "nuvoton,npcm-kcs-bmc"

If we use "nuvoton, npcm-kcs-bmc" we should take care of backward dts compatibility, and I am not sure we like to change NPCM KCS driver.

Quoted 10 lines

If the device has a minor difference that can be expressed in a parameter, then create a parameter for it.

If the device has enough differences that a parameter or two doesn't cover it, then you put either nuvoton,npcm750-kcs-bmc or nuvoton,npcm750-kcs-bmc in the device tree. Not both. Then you need two entries in the of_device_id array and you use the data field or something to express the difference.

Quoted 7 lines

Since there appears to be no difference, just put "nuvoton,npcm750-kcs-bmc" in the npcm845 and I will drop the patch adding all this. Then a patch can be added saying it applies to both the 7xx and 8xx series of BMC SOCs. If you want to change the name, then a patch will be needed for that, but then you will need multiple entries in your device tree, but you would not document it as such, as there would only be one that applies for this kernel.

Quoted 2 lines

It little bit confusing to use nuvoton,npcm750-kcs-bmc that are related to NPCM7XX for NPCM8XX KCS.

A little, but it's not unusual.

Quoted 10 lines

We can use the generic name "nuvoton, npcm-kcs-bmc" as you suggested above but we should take care of backward dts compatibility, and I am not sure we like to change NPCM KCS driver.

We had a disscation with Arnd, Arnd asked us to use a fallback as we did here if NPCM8XX device module is similar to NPCM7XX module:
https://lore.kernel.org/lkml/[email protected]/

I think we should use a fallback to describe the NPCM8XX KCS in the dt-binding document.

I'm ok with that option. I guess I should have mentioned it. Add nuvoton,npcm-kcs-bmc to the driver's of_device_id table. Then use that in that compatible string in the device tree. Leave the 750 compatible string in the table for backwards compatibility.

There's no point in having a bunch of those strings if they are all the same. If a new one comes out that is different, we can handle that when the time comes.

-corey

Quoted 23 lines
Quoted 19 lines

I'm pretty sure the only reason to have muliple compatible entries in a device tree is to cover multiple kernels where the name changed.

-corey

Quoted 13 lines
Quoted 9 lines

-corey

Quoted 6 lines
Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt +0 -1
 - interrupts : interrupt generated by the controller
 - kcs_chan : The KCS channel number in the controller
 
--

2.33.0

Best regards,

Tomer

Best regards,

Tomer

#10 lore raw
Quoted 9 lines · Corey Minyard
Quoted 6 lines

We had a disscation with Arnd, Arnd asked us to use a fallback as we did here if NPCM8XX device module is similar to NPCM7XX module:
https://lore.kernel.org/lkml/[email protected]/

I think we should use a fallback to describe the NPCM8XX KCS in the dt-binding document.

I'm ok with that option. I guess I should have mentioned it. Add nuvoton,npcm-kcs-bmc to the driver's of_device_id table.

To be clear - NAK. It's not a specific compatible, therefore it is not appropriate at all.

Quoted 7 lines

Then use that
in that compatible string in the device tree. Leave the 750 compatible string in the table for backwards compatibility.

There's no point in having a bunch of those strings if they are all the same. If a new one comes out that is different, we can handle that when the time comes.\

There is a point. It's exactly how we do in all DT bindings. It is recommended by writing bindings document.

Best regards,
Krzysztof

#11 lore raw
In reply to Tomer Maimon
Quoted 62 lines · Tomer Maimon
Quoted 35 lines · Corey Minyard
Quoted 30 lines · Tomer Maimon

Hi Corey,

Thanks for your comment.

Quoted 10 lines · Corey Minyard
Quoted 8 lines · Tomer Maimon

Add to npcm845 KCS compatible string a fallback to npcm750 KCS compatible string becuase NPCM845 and NPCM750 BMCs are using identical KCS modules.

Signed-off-by Tomer Maimon <[email protected]>
---
 Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 15 lines

Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt used to perform in-band IPMI communication with their host. · line 10

Quoted 10 lines
Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt +1 -1
index cbc10a68ddef..4fda76e63396 100644
@@ -7,7 +7,7 @@ used to perform in-band IPMI communication with their host.
 Required properties:
 - compatible : should be one of
     "nuvoton,npcm750-kcs-bmc"
-    "nuvoton,npcm845-kcs-bmc"
+    "nuvoton,npcm845-kcs-bmc", "nuvoton,npcm750-kcs-bmc"

This is just wrong. The compatible is supposed to identify the device, not the board the device is on. I think compatible here should be "npcm7xx-kcs-bmc", and just use that everywhere. It's fine if that is used on a board named npcm845.

Quoted 2 lines

The NPCM8XX is not a board, The Nuvoton NPCM8XX is a fourth-generation BMC SoC device family.

Quoted 6 lines

Ok, but same principle applies.

If the device is exactly the same, then you would only use one of the "npcm7xx-kcs-bmc" and put that in both device trees. You can use "nuvoton,npcm750-kcs-bmc", it's really not that important. Or even "nuvoton,npcm-kcs-bmc"

If we use "nuvoton, npcm-kcs-bmc" we should take care of backward dts compatibility, and I am not sure we like to change NPCM KCS driver.

Quoted 10 lines

If the device has a minor difference that can be expressed in a parameter, then create a parameter for it.

If the device has enough differences that a parameter or two doesn't cover it, then you put either nuvoton,npcm750-kcs-bmc or nuvoton,npcm750-kcs-bmc in the device tree. Not both. Then you need two entries in the of_device_id array and you use the data field or something to express the difference.

Quoted 7 lines

Since there appears to be no difference, just put "nuvoton,npcm750-kcs-bmc" in the npcm845 and I will drop the patch adding all this. Then a patch can be added saying it applies to both the 7xx and 8xx series of BMC SOCs. If you want to change the name, then a patch will be needed for that, but then you will need multiple entries in your device tree, but you would not document it as such, as there would only be one that applies for this kernel.

Quoted 3 lines

It little bit confusing to use nuvoton,npcm750-kcs-bmc that are related to NPCM7XX for NPCM8XX KCS.
We can use the generic name "nuvoton, npcm-kcs-bmc" as you suggested

No, please don't. It will be NAKed. :)

Quoted 9 lines

above but we should take care of backward dts compatibility, and I am not sure we like to change NPCM KCS driver.

We had a disscation with Arnd, Arnd asked us to use a fallback as we did here if NPCM8XX device module is similar to NPCM7XX module:
https://lore.kernel.org/lkml/[email protected]/

I think we should use a fallback to describe the NPCM8XX KCS in the dt-binding document.

Best regards,
Krzysztof

#12 lore raw
In reply to Corey Minyard
Quoted 34 lines · Corey Minyard
Quoted 30 lines · Tomer Maimon

Hi Corey,

Thanks for your comment.

Quoted 10 lines · Corey Minyard
Quoted 8 lines · Tomer Maimon

Add to npcm845 KCS compatible string a fallback to npcm750 KCS compatible string becuase NPCM845 and NPCM750 BMCs are using identical KCS modules.

Signed-off-by Tomer Maimon <[email protected]>
---
 Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 15 lines

Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt used to perform in-band IPMI communication with their host. · line 10

Quoted 10 lines
Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt +1 -1
index cbc10a68ddef..4fda76e63396 100644
@@ -7,7 +7,7 @@ used to perform in-band IPMI communication with their host.
 Required properties:
 - compatible : should be one of
     "nuvoton,npcm750-kcs-bmc"
-    "nuvoton,npcm845-kcs-bmc"
+    "nuvoton,npcm845-kcs-bmc", "nuvoton,npcm750-kcs-bmc"

This is just wrong. The compatible is supposed to identify the device, not the board the device is on. I think compatible here should be "npcm7xx-kcs-bmc", and just use that everywhere. It's fine if that is used on a board named npcm845.

Quoted 2 lines

The NPCM8XX is not a board, The Nuvoton NPCM8XX is a fourth-generation BMC SoC device family.

Quoted 6 lines

Ok, but same principle applies.

If the device is exactly the same, then you would only use one of the "npcm7xx-kcs-bmc" and put that in both device trees. You can use "nuvoton,npcm750-kcs-bmc", it's really not that important. Or even "nuvoton,npcm-kcs-bmc"

No, because it is too generic. Compatibles must be specific.

Quoted 9 lines

If the device has a minor difference that can be expressed in a parameter, then create a parameter for it.

If the device has enough differences that a parameter or two doesn't cover it, then you put either nuvoton,npcm750-kcs-bmc or nuvoton,npcm750-kcs-bmc in the device tree. Not both. Then you need two entries in the of_device_id array and you use the data field or something to express the difference.

It's quite common to have generic and specific compatibles for compatible devices and a driver which can match to both of them. I don't understand where is exactly the problem here?

Quoted 4 lines

Since there appears to be no difference, just put "nuvoton,npcm750-kcs-bmc" in the npcm845 and I will drop the patch adding all this. T

Again no, because recommended (also writing bindings document) is always to have a specific compatible.

Quoted 8 lines

hen a patch can be added saying it applies to both
the 7xx and 8xx series of BMC SOCs. If you want to change the name, then a patch will be needed for that, but then you will need multiple entries in your device tree, but you would not document it as such, as there would only be one that applies for this kernel.

I'm pretty sure the only reason to have muliple compatible entries in a device tree is to cover multiple kernels where the name changed.

Best regards,
Krzysztof

#13 lore raw
In reply to Corey Minyard
Quoted 9 lines · Corey Minyard
Quoted 8 lines · Tomer Maimon

Add to npcm845 KCS compatible string a fallback to npcm750 KCS compatible string becuase NPCM845 and NPCM750 BMCs are using identical KCS modules.

Signed-off-by Tomer Maimon <[email protected]>
---
 Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 14 lines

Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt used to perform in-band IPMI communication with their host. · line 10

Quoted 10 lines
Documentation/devicetree/bindings/ipmi/npcm7xx-kcs-bmc.txt +1 -1
index cbc10a68ddef..4fda76e63396 100644
@@ -7,7 +7,7 @@ used to perform in-band IPMI communication with their host.
 Required properties:
 - compatible : should be one of
     "nuvoton,npcm750-kcs-bmc"
-    "nuvoton,npcm845-kcs-bmc"
+    "nuvoton,npcm845-kcs-bmc", "nuvoton,npcm750-kcs-bmc"

This is just wrong. The compatible is supposed to identify the device, not the board the device is on. I think compatible here should be "npcm7xx-kcs-bmc", and just use that everywhere. It's fine if that is

No, because you propose to use wildcards as compatible which is not correct. Compatibles must be specific.

Best regards,
Krzysztof