Skip to contentSkip to conversation

leds: pca955x: Add IBM software implemenation of the PCA9552 chip

PATCH Superseded Eddie James 4 participants 8 messages Jul 9, 2020 – Jul 20, 2020

Subsystems (2)
Device tree bindings LED

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Eddie James

author
#0 lore raw

cover leds: pca955x: Add IBM software implemenation of the PCA9552 chip

IBM created an implementation of the PCA9552 on a PIC16F microcontroller. The I2C device addresses are different from the hardware PCA9552, so add a new compatible string and associated platform data to be able to probe this device, and document the new string.

Eddie James (2):
dt-bindings: leds: pca955x: Add IBM implementation compatible string
leds: pca955x: Add an IBM software implementation of the PCA9552 chip

 Documentation/devicetree/bindings/leds/leds-pca955x.txt | 1 +
 drivers/leds/leds-pca955x.c                             | 8 ++++++++
 2 files changed, 9 insertions(+)
Signature · 1 line
2.24.0
author
#1 lore raw

PATCH 1/2 dt-bindings: leds: pca955x: Add IBM implementation compatible string

IBM created an implementation of the PCA9552 on a PIC16F microcontroller. Document the new compatible string for this device.

Signed-off-by Eddie James <[email protected]>
---
 Documentation/devicetree/bindings/leds/leds-pca955x.txt | 1 +
 1 file changed, 1 insertion(+)
Documentation/devicetree/bindings/leds/leds-pca955x.txt +1 -0
index 7a5830f8d5ab..28edb976ee77 100644
@@ -9,6 +9,7 @@ Required properties:
 	"nxp,pca9550"
 	"nxp,pca9551"
 	"nxp,pca9552"
+	"nxp,pca9552-ibm"
 	"nxp,pca9553"
 - #address-cells: must be 1
 - #size-cells: must be 0
Signature · 1 line
2.24.0
#2 lore raw

Re: [PATCH 1/2] dt-bindings: leds: pca955x: Add IBM implementation compatible string

Hi!

Quoted 2 lines

IBM created an implementation of the PCA9552 on a PIC16F microcontroller. Document the new compatible string for this device.

Is the implementation opensource?

Documentation/devicetree/bindings/leds/leds-pca955x.txt Required properties: · line 13

Quoted 9 lines
Signed-off-by Eddie James <[email protected]>

+++ b/Documentation/devicetree/bindings/leds/leds-pca955x.txt

Documentation/devicetree/bindings/leds/leds-pca955x.txt +1 -0
@@ -9,6 +9,7 @@ Required properties:
 	"nxp,pca9550"
 	"nxp,pca9551"
 	"nxp,pca9552"
+	"nxp,pca9552-ibm"
 	"nxp,pca9553"

Is it good idea to use nxp prefix for something that is software-defined and not built by nxp?

Would ibm,pca9552 be better, or maybe even sw,pca9552 to indicate that is not real hardware, but software emulation?

Best regards,
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#3 lore raw

Re: [PATCH 1/2] dt-bindings: leds: pca955x: Add IBM implementation compatible string

Quoted 5 lines · Pavel Machek

Hi!

Quoted 2 lines

IBM created an implementation of the PCA9552 on a PIC16F microcontroller. Document the new compatible string for this device.

Is the implementation opensource?

Hi, no it is not.

Quoted 11 lines

Documentation/devicetree/bindings/leds/leds-pca955x.txt Required properties: · line 13

Quoted 9 lines
Signed-off-by Eddie James <[email protected]>

+++ b/Documentation/devicetree/bindings/leds/leds-pca955x.txt

Documentation/devicetree/bindings/leds/leds-pca955x.txt +1 -0
@@ -9,6 +9,7 @@ Required properties:
  	"nxp,pca9550"
  	"nxp,pca9551"
  	"nxp,pca9552"
+	"nxp,pca9552-ibm"
  	"nxp,pca9553"

Is it good idea to use nxp prefix for something that is software-defined and not built by nxp?

Yea I suppose not...

Quoted 3 lines

Would ibm,pca9552 be better, or maybe even sw,pca9552 to indicate that is not real hardware, but software emulation?

How about ibm,pca9552-sw? Someone suggested that just adding "sw" could be a problem if another company does the same thing but it isn't compatible.

Thanks for taking a look!

Eddie

Quoted 3 lines

Best regards,
Pavel

maintainer
#4 lore raw

Re: [PATCH 1/2] dt-bindings: leds: pca955x: Add IBM implementation compatible string

Quoted 28 lines · Eddie James
Quoted 5 lines · Pavel Machek

Hi!

Quoted 2 lines

IBM created an implementation of the PCA9552 on a PIC16F microcontroller. Document the new compatible string for this device.

Is the implementation opensource?

Hi, no it is not.

Quoted 11 lines

Documentation/devicetree/bindings/leds/leds-pca955x.txt Required properties: · line 13

Quoted 9 lines
Signed-off-by Eddie James <[email protected]>

+++ b/Documentation/devicetree/bindings/leds/leds-pca955x.txt

Documentation/devicetree/bindings/leds/leds-pca955x.txt +1 -0
@@ -9,6 +9,7 @@ Required properties:
  	"nxp,pca9550"
  	"nxp,pca9551"
  	"nxp,pca9552"
+	"nxp,pca9552-ibm"
  	"nxp,pca9553"

Is it good idea to use nxp prefix for something that is software-defined and not built by nxp?

Yea I suppose not...

Quoted 1 line
Quoted 6 lines
Quoted 2 lines

Would ibm,pca9552 be better, or maybe even sw,pca9552 to indicate that is not real hardware, but software emulation?

How about ibm,pca9552-sw? Someone suggested that just adding "sw" could be a problem if another company does the same thing but it isn't compatible.

ibm,pca9552 is good.

Rob

author
#5 lore raw

PATCH 2/2 leds: pca955x: Add an IBM software implementation of the PCA9552 chip

In reply to Eddie James

IBM created an implementation of the PCA9552 on a PIC16F microcontroller. The I2C device addresses are different from the hardware PCA9552, so add a new compatible string and associated platform data to be able to probe this device.

Signed-off-by Eddie James <[email protected]>
Reviewed-by Vishwanatha Subbanna <[email protected]>
---
 drivers/leds/leds-pca955x.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/leds/leds-pca955x.c +8 -0
index 4037c504589c..bf7ead45f66b 100644
@@ -65,6 +65,7 @@ enum pca955x_type {
 	pca9550,
 	pca9551,
 	pca9552,
+	pca9552_ibm,
 	pca9553,
 };
 

@@ -90,6 +91,11 @@ static struct pca955x_chipdef pca955x_chipdefs[] = {
 		.slv_addr	= /* 1100xxx */ 0x60,
 		.slv_addr_shift	= 3,
 	},
+	[pca9552_ibm] = {
+		.bits		= 16,
+		.slv_addr	= /* 0110xxx */ 0x30,
+		.slv_addr_shift	= 3,
+	},
 	[pca9553] = {
 		.bits		= 4,
 		.slv_addr	= /* 110001x */ 0x62,

@@ -101,6 +107,7 @@ static const struct i2c_device_id pca955x_id[] = {
 	{ "pca9550", pca9550 },
 	{ "pca9551", pca9551 },
 	{ "pca9552", pca9552 },
+	{ "pca9552-ibm", pca9552_ibm },
 	{ "pca9553", pca9553 },
 	{ }
 };

@@ -412,6 +419,7 @@ static const struct of_device_id of_pca955x_match[] = {
 	{ .compatible = "nxp,pca9550", .data = (void *)pca9550 },
 	{ .compatible = "nxp,pca9551", .data = (void *)pca9551 },
 	{ .compatible = "nxp,pca9552", .data = (void *)pca9552 },
+	{ .compatible = "nxp,pca9552-ibm", .data = (void *)pca9552_ibm },
 	{ .compatible = "nxp,pca9553", .data = (void *)pca9553 },
 	{},
 };
Signature · 1 line
2.24.0
#6 lore raw

Re: [PATCH 2/2] leds: pca955x: Add an IBM software implementation of the PCA9552 chip

Quoted 5 lines · Eddie James

IBM created an implementation of the PCA9552 on a PIC16F microcontroller. The I2C device addresses are different from the hardware PCA9552, so add a new compatible string and associated platform data to be able to probe this device.

This is weird. I would rather expect ibm prefix with corresponding part number.

Quoted 7 lines
(quoted diff) +4 -0
+       pca9552_ibm,
 
+       [pca9552_ibm] = {
 
+       { "pca9552-ibm", pca9552_ibm },
 
+       { .compatible = "nxp,pca9552-ibm", .data = (void *)pca9552_ibm },
Signature · 2 lines
With Best Regards,
Andy Shevchenko
author
#7 lore raw

Re: [PATCH 2/2] leds: pca955x: Add an IBM software implementation of the PCA9552 chip

Quoted 6 lines · Andy Shevchenko
Quoted 4 lines · Eddie James

IBM created an implementation of the PCA9552 on a PIC16F microcontroller. The I2C device addresses are different from the hardware PCA9552, so add a new compatible string and associated platform data to be able to probe this device.

This is weird. I would rather expect ibm prefix with corresponding part number.

Yep I agree now, see my note to Pavel just now.

Thanks,

Eddie

Quoted 6 lines
Quoted 5 lines
(quoted diff) +4 -0
+       pca9552_ibm,
+       [pca9552_ibm] = {
+       { "pca9552-ibm", pca9552_ibm },
+       { .compatible = "nxp,pca9552-ibm", .data = (void *)pca9552_ibm },