[PATCH linux dev-5.4] leds: pca955x: Add a software implementation of the PCA9552 chip

[PATCH linux dev-5.4] leds: pca955x: Add a software implementation of the PCA9552 chip

[Eddie James]

There is 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 <eajames@linux.ibm.com>
---
 drivers/leds/leds-pca955x.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c
index 4037c504589c..4d3a36538c6c 100644
--- a/drivers/leds/leds-pca955x.c
+++ b/drivers/leds/leds-pca955x.c
@@ -65,6 +65,7 @@ enum pca955x_type {
 	pca9550,
 	pca9551,
 	pca9552,
+	pca9552_soft,
 	pca9553,
 };
 
@@ -90,6 +91,11 @@ static struct pca955x_chipdef pca955x_chipdefs[] = {
 		.slv_addr	= /* 1100xxx */ 0x60,
 		.slv_addr_shift	= 3,
 	},
+	[pca9552_soft] = {
+		.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-soft", pca9552_soft },
 	{ "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-soft", .data = (void *)pca9552_soft },
 	{ .compatible = "nxp,pca9553", .data = (void *)pca9553 },
 	{},
 };
-- 
2.24.0

Re: [PATCH linux dev-5.4] leds: pca955x: Add a software implementation of the PCA9552 chip

[Vishwanatha Subbanna]

> On 22-Jun-2020, at 9:15 PM, Eddie James <eajames@linux.ibm.com> wrote:
> 
> There is 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 <eajames@linux.ibm.com>
> ---
> drivers/leds/leds-pca955x.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c
> index 4037c504589c..4d3a36538c6c 100644
> --- a/drivers/leds/leds-pca955x.c
> +++ b/drivers/leds/leds-pca955x.c
> @@ -65,6 +65,7 @@ enum pca955x_type {
> 	pca9550,
> 	pca9551,
> 	pca9552,
> +	pca9552_soft,
> 	pca9553,
> };
> 
> @@ -90,6 +91,11 @@ static struct pca955x_chipdef pca955x_chipdefs[] = {
> 		.slv_addr	= /* 1100xxx */ 0x60,
> 		.slv_addr_shift	= 3,
> 	},
> +	[pca9552_soft] = {
> +		.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-soft", pca9552_soft },
> 	{ "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-soft", .data = (void *)pca9552_soft },
> 	{ .compatible = "nxp,pca9553", .data = (void *)pca9553 },
> 	{},
> };
> — 
> 2.24.0
> 

This looks good to me and I have tested it successfully.

Reviewed-by: Vishwanatha Subbanna <vishwa@linux.vnet.ibm.com> 

Re: [PATCH linux dev-5.4] leds: pca955x: Add a software implementation of the PCA9552 chip

[Andrew Jeffery]

On Tue, 23 Jun 2020, at 01:15, Eddie James wrote:
> There is 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 <eajames@linux.ibm.com>

I think we need to bikeshed the name a bit, and that might be best done 
upstream. I feel like we should be putting "ibm" in the name to avoid the 
haziness of "soft". Another company could do the same thing with different 
choices for things like the device address parameters.

Andrew