Re: [PATCH 2/2] leds: rgb: leds-ktd202x: Get device properties through fwnode to support ACPI

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

Am Mittwoch, dem 07.02.2024 um 14:10 +0800 schrieb Kate Hsuan:
> This LED controller also installed on a Xiaomi pad2 and it is an x86
> platform. The original driver is based on the device tree and can't
> be used for this ACPI based system. This patch migrated the driver to
> use fwnode to access the properties. Moreover, the fwnode API
> supports device tree so this work would affect the original
would -> won't ?

> implementations.
> 
> Signed-off-by: Kate Hsuan <hpa@xxxxxxxxxx>
> ---
>  drivers/leds/rgb/Kconfig        |  1 -
>  drivers/leds/rgb/leds-ktd202x.c | 68 ++++++++++++++++++++++---------
> --
>  2 files changed, 45 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/leds/rgb/Kconfig b/drivers/leds/rgb/Kconfig
> index a6a21f564673..f245dbd9a163 100644
> --- a/drivers/leds/rgb/Kconfig
> +++ b/drivers/leds/rgb/Kconfig
> @@ -17,7 +17,6 @@ config LEDS_GROUP_MULTICOLOR
>  config LEDS_KTD202X
>  	tristate "LED support for KTD202x Chips"
>  	depends on I2C
> -	depends on OF
>  	select REGMAP_I2C
>  	help
>  	  This option enables support for the Kinetic
> KTD2026/KTD2027
> diff --git a/drivers/leds/rgb/leds-ktd202x.c b/drivers/leds/rgb/leds-
> ktd202x.c
> index 514965795a10..a1aef62e3db5 100644
> --- a/drivers/leds/rgb/leds-ktd202x.c
> +++ b/drivers/leds/rgb/leds-ktd202x.c
> @@ -112,6 +112,8 @@ static int ktd202x_chip_disable(struct ktd202x
> *chip)
>  
>  	regmap_write(chip->regmap, KTD202X_REG_RESET_CONTROL,
> KTD202X_ENABLE_CTRL_SLEEP);
>  
> +	usleep_range(500, 600);
Why have you added the sleep?

> +
>  	ret = regulator_bulk_disable(ARRAY_SIZE(chip->regulators),
> chip->regulators);
>  	if (ret) {
>  		dev_err(chip->dev, "Failed to disable regulators:
> %d\n", ret);
> @@ -381,16 +383,18 @@ static int ktd202x_blink_mc_set(struct
> led_classdev *cdev,
>  				 mc->num_colors);
>  }
>  
> -static int ktd202x_setup_led_rgb(struct ktd202x *chip, struct
> device_node *np,
> +static int ktd202x_setup_led_rgb(struct ktd202x *chip, struct
> fwnode_handle *np,
>  				 struct ktd202x_led *led, struct
> led_init_data *init_data)
>  {
>  	struct led_classdev *cdev;
> -	struct device_node *child;
> +	struct fwnode_handle *child;
Nit, swap the above lines for reverse Christmas tree.

>  	struct mc_subled *info;
> -	int num_channels;
> +	int num_channels = 0;
>  	int i = 0;
>  
> -	num_channels = of_get_available_child_count(np);
> +	fwnode_for_each_available_child_node(np, child) {
> +		num_channels++;
> +	}
>  	if (!num_channels || num_channels > chip->num_leds)
>  		return -EINVAL;
>  
> @@ -398,22 +402,22 @@ static int ktd202x_setup_led_rgb(struct ktd202x
> *chip, struct device_node *np,
>  	if (!info)
>  		return -ENOMEM;
>  
> -	for_each_available_child_of_node(np, child) {
> +	fwnode_for_each_available_child_node(np, child) {
>  		u32 mono_color;
>  		u32 reg;
>  		int ret;
>  
> -		ret = of_property_read_u32(child, "reg", &reg);
> +		ret = fwnode_property_read_u32(child, "reg", &reg);
>  		if (ret != 0 || reg >= chip->num_leds) {
>  			dev_err(chip->dev, "invalid 'reg' of
> %pOFn\n", child);
> -			of_node_put(child);
> +			fwnode_handle_put(child);
>  			return -EINVAL;
>  		}
>  
> -		ret = of_property_read_u32(child, "color",
> &mono_color);
> +		ret = fwnode_property_read_u32(child, "color",
> &mono_color);
>  		if (ret < 0 && ret != -EINVAL) {
>  			dev_err(chip->dev, "failed to parse 'color'
> of %pOF\n", child);
> -			of_node_put(child);
> +			fwnode_handle_put(child);
>  			return ret;
>  		}
>  
> @@ -433,14 +437,14 @@ static int ktd202x_setup_led_rgb(struct ktd202x
> *chip, struct device_node *np,
>  	return devm_led_classdev_multicolor_register_ext(chip->dev,
> &led->mcdev, init_data);
>  }
>  
> -static int ktd202x_setup_led_single(struct ktd202x *chip, struct
> device_node *np,
> +static int ktd202x_setup_led_single(struct ktd202x *chip, struct
> fwnode_handle *np,
>  				    struct ktd202x_led *led, struct
> led_init_data *init_data)
>  {
>  	struct led_classdev *cdev;
>  	u32 reg;
>  	int ret;
>  
> -	ret = of_property_read_u32(np, "reg", &reg);
> +	ret = fwnode_property_read_u32(np, "reg", &reg);
>  	if (ret != 0 || reg >= chip->num_leds) {
>  		dev_err(chip->dev, "invalid 'reg' of %pOFn\n", np);
>  		return -EINVAL;
> @@ -454,7 +458,7 @@ static int ktd202x_setup_led_single(struct
> ktd202x *chip, struct device_node *np
>  	return devm_led_classdev_register_ext(chip->dev, &led->cdev,
> init_data);
>  }
>  
> -static int ktd202x_add_led(struct ktd202x *chip, struct device_node
> *np, unsigned int index)
> +static int ktd202x_add_led(struct ktd202x *chip, struct
> fwnode_handle *np, unsigned int index)
>  {
>  	struct ktd202x_led *led = &chip->leds[index];
>  	struct led_init_data init_data = {};
> @@ -463,14 +467,14 @@ static int ktd202x_add_led(struct ktd202x
> *chip, struct device_node *np, unsigne
>  	int ret;
>  
>  	/* Color property is optional in single color case */
> -	ret = of_property_read_u32(np, "color", &color);
> +	ret = fwnode_property_read_u32(np, "color", &color);
>  	if (ret < 0 && ret != -EINVAL) {
>  		dev_err(chip->dev, "failed to parse 'color' of
> %pOF\n", np);
>  		return ret;
>  	}
>  
>  	led->chip = chip;
> -	init_data.fwnode = of_fwnode_handle(np);
> +	init_data.fwnode = np;
>  
>  	if (color == LED_COLOR_ID_RGB) {
>  		cdev = &led->mcdev.led_cdev;
> @@ -492,26 +496,30 @@ static int ktd202x_add_led(struct ktd202x
> *chip, struct device_node *np, unsigne
>  
>  static int ktd202x_probe_dt(struct ktd202x *chip)
>  {
> -	struct device_node *np = dev_of_node(chip->dev), *child;
> -	int count;
> +	struct device *dev = chip->dev;
> +	struct fwnode_handle *np, *child;
Nit, swap the above lines.

> +	int count = 0;
Un-needed init.

>  	int i = 0;
>  
> -	chip->num_leds = (int)(unsigned
> long)of_device_get_match_data(chip->dev);
> +	count = device_get_child_node_count(dev);
>  
> -	count = of_get_available_child_count(np);
>  	if (!count || count > chip->num_leds)
>  		return -EINVAL;
>  
> +	np = dev_fwnode(chip->dev);
> +	if (!np)
> +		return -ENODEV;
> +
>  	regmap_write(chip->regmap, KTD202X_REG_RESET_CONTROL,
> KTD202X_RSTR_RESET);
>  
>  	/* Allow the device to execute the complete reset */
>  	usleep_range(200, 300);
>  
> -	for_each_available_child_of_node(np, child) {
> +	fwnode_for_each_available_child_node(np, child) {
>  		int ret = ktd202x_add_led(chip, child, i);
>  
>  		if (ret) {
> -			of_node_put(child);
> +			fwnode_handle_put(child);
>  			return ret;
>  		}
>  		i++;
> @@ -533,7 +541,7 @@ static int ktd202x_probe(struct i2c_client
> *client)
>  {
>  	struct device *dev = &client->dev;
>  	struct ktd202x *chip;
> -	int count;
> +	int count = 0;
Un-needed init.

>  	int ret;
>  
>  	count = device_get_child_node_count(dev);
> @@ -568,6 +576,8 @@ static int ktd202x_probe(struct i2c_client
> *client)
>  		return ret;
>  	}
>  
> +	chip->num_leds = (int) (unsigned
> long)i2c_get_match_data(client);
> +
>  	ret = ktd202x_probe_dt(chip);
>  	if (ret < 0) {
>  		regulator_bulk_disable(ARRAY_SIZE(chip->regulators),
> chip->regulators);
> @@ -602,21 +612,33 @@ static void ktd202x_shutdown(struct i2c_client
> *client)
>  	regmap_write(chip->regmap, KTD202X_REG_RESET_CONTROL,
> KTD202X_RSTR_RESET);
>  }
>  
> +static const struct i2c_device_id ktd202x_id[] = {
> +	{"ktd2026", KTD2026_NUM_LEDS},
> +	{"ktd2027", KTD2027_NUM_LEDS},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(i2c, ktd202x_id);
> +
> +#ifndef CONFIG_ACPI
>  static const struct of_device_id ktd202x_match_table[] = {
>  	{ .compatible = "kinetic,ktd2026", .data = (void
> *)KTD2026_NUM_LEDS },
>  	{ .compatible = "kinetic,ktd2027", .data = (void
> *)KTD2027_NUM_LEDS },
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, ktd202x_match_table);
> +#endif
>  
>  static struct i2c_driver ktd202x_driver = {
>  	.driver = {
> -		.name = "leds-ktd202x",
> -		.of_match_table = ktd202x_match_table,
> +		.name	= "leds-ktd202x",
Why was the space after "name" replaced by tab?

> +#ifndef CONFIG_ACPI
> +		.of_match_table = ktd20xx_match_table,
Typo: ktd20xx_match_table -> ktd202x_match_table

André

> +#endif
>  	},
>  	.probe = ktd202x_probe,
>  	.remove = ktd202x_remove,
>  	.shutdown = ktd202x_shutdown,
> +	.id_table = ktd202x_id,
>  };
>  module_i2c_driver(ktd202x_driver);
>  






[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux