Re: [PATCH v1] leds: pca955x: Make use of device property API

[Jacek Anaszewski <jacek.anaszewski@xxxxxxxxx>]

Hi Andy,

After removing ACPI support these patches don't apply.
Actually, did you change your mind about supporting
unofficial ACPI ids?

Best regards,
Jacek Anaszewski

On 3/18/19 6:22 PM, Andy Shevchenko wrote:
> Make use of device property API in this driver so that both OF based
> system and ACPI based system can use this driver.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> ---
>   drivers/leds/leds-pca955x.c | 61 ++++++++++++++++---------------------
>   1 file changed, 26 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c
> index f51b356d4426..25ad57fc3866 100644
> --- a/drivers/leds/leds-pca955x.c
> +++ b/drivers/leds/leds-pca955x.c
> @@ -48,8 +48,8 @@
>   #include <linux/i2c.h>
>   #include <linux/leds.h>
>   #include <linux/module.h>
> -#include <linux/of_device.h>
>   #include <linux/of.h>
> +#include <linux/property.h>
>   #include <linux/slab.h>
>   #include <linux/string.h>
>   
> @@ -110,6 +110,15 @@ static const struct i2c_device_id pca955x_id[] = {
>   };
>   MODULE_DEVICE_TABLE(i2c, pca955x_id);
>   
> +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,pca9553", .data = (void *)pca9553 },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, of_pca955x_match);
> +
>   static const struct acpi_device_id pca955x_acpi_ids[] = {
>   	{ "PCA9550",  pca9550 },
>   	{ "PCA9551",  pca9551 },
> @@ -373,16 +382,14 @@ static int pca955x_gpio_direction_output(struct gpio_chip *gc,
>   }
>   #endif /* CONFIG_LEDS_PCA955X_GPIO */
>   
> -#if IS_ENABLED(CONFIG_OF)
>   static struct pca955x_platform_data *
> -pca955x_pdata_of_init(struct i2c_client *client, struct pca955x_chipdef *chip)
> +pca955x_get_pdata(struct i2c_client *client, struct pca955x_chipdef *chip)
>   {
> -	struct device_node *np = client->dev.of_node;
> -	struct device_node *child;
>   	struct pca955x_platform_data *pdata;
> +	struct fwnode_handle *child;
>   	int count;
>   
> -	count = of_get_child_count(np);
> +	count = device_get_child_node_count(&client->dev);
>   	if (!count || count > chip->bits)
>   		return ERR_PTR(-ENODEV);
>   
> @@ -396,24 +403,25 @@ pca955x_pdata_of_init(struct i2c_client *client, struct pca955x_chipdef *chip)
>   	if (!pdata->leds)
>   		return ERR_PTR(-ENOMEM);
>   
> -	for_each_child_of_node(np, child) {
> +	device_for_each_child_node(&client->dev, child) {
>   		const char *name;
>   		u32 reg;
>   		int res;
>   
> -		res = of_property_read_u32(child, "reg", &reg);
> +		res = fwnode_property_read_u32(child, "reg", &reg);
>   		if ((res != 0) || (reg >= chip->bits))
>   			continue;
>   
> -		if (of_property_read_string(child, "label", &name))
> -			name = child->name;
> +		res = fwnode_property_read_string(child, "label", &name);
> +		if ((res != 0) && is_of_node(child))
> +			name = to_of_node(child)->name;
>   
>   		snprintf(pdata->leds[reg].name, sizeof(pdata->leds[reg].name),
>   			 "%s", name);
>   
>   		pdata->leds[reg].type = PCA955X_TYPE_LED;
> -		of_property_read_u32(child, "type", &pdata->leds[reg].type);
> -		of_property_read_string(child, "linux,default-trigger",
> +		fwnode_property_read_u32(child, "type", &pdata->leds[reg].type);
> +		fwnode_property_read_string(child, "linux,default-trigger",
>   					&pdata->leds[reg].default_trigger);
>   	}
>   
> @@ -422,23 +430,6 @@ pca955x_pdata_of_init(struct i2c_client *client, struct pca955x_chipdef *chip)
>   	return pdata;
>   }
>   
> -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,pca9553", .data = (void *)pca9553 },
> -	{},
> -};
> -
> -MODULE_DEVICE_TABLE(of, of_pca955x_match);
> -#else
> -static struct pca955x_platform_data *
> -pca955x_pdata_of_init(struct i2c_client *client, struct pca955x_chipdef *chip)
> -{
> -	return ERR_PTR(-ENODEV);
> -}
> -#endif
> -
>   static int pca955x_probe(struct i2c_client *client,
>   					const struct i2c_device_id *id)
>   {
> @@ -453,17 +444,17 @@ static int pca955x_probe(struct i2c_client *client,
>   	if (id) {
>   		chip = &pca955x_chipdefs[id->driver_data];
>   	} else {
> -		const struct acpi_device_id *acpi_id;
> +		const void *match;
>   
> -		acpi_id = acpi_match_device(pca955x_acpi_ids, &client->dev);
> -		if (!acpi_id)
> +		match = device_get_match_data(&client->dev);
> +		if (!match)
>   			return -ENODEV;
> -		chip = &pca955x_chipdefs[acpi_id->driver_data];
> +		chip = &pca955x_chipdefs[(enum pca955x_type)match];
>   	}
>   	adapter = to_i2c_adapter(client->dev.parent);
>   	pdata = dev_get_platdata(&client->dev);
>   	if (!pdata) {
> -		pdata =	pca955x_pdata_of_init(client, chip);
> +		pdata =	pca955x_get_pdata(client, chip);
>   		if (IS_ERR(pdata))
>   			return PTR_ERR(pdata);
>   	}
> @@ -602,8 +593,8 @@ static int pca955x_probe(struct i2c_client *client,
>   static struct i2c_driver pca955x_driver = {
>   	.driver = {
>   		.name	= "leds-pca955x",
> +		.of_match_table = of_pca955x_match,
>   		.acpi_match_table = ACPI_PTR(pca955x_acpi_ids),
> -		.of_match_table = of_match_ptr(of_pca955x_match),
>   	},
>   	.probe	= pca955x_probe,
>   	.id_table = pca955x_id,