Re: [PATCH 3/3] Input: soc_button_array - convert to software nodes

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Tue, Oct 20, 2026 at 06:50:01PM -0700, Dmitry Torokhov wrote:
> Convert the driver from using legacy platform data to using software
> nodes for registering 'gpio-keys' devices. This allows us to
> transition away from platform data and use generic device properties.

...

> +static void soc_button_remove_swnodes(struct fwnode_handle *parent)
> +{
> +	struct fwnode_handle *child;

> +	if (IS_ERR_OR_NULL(parent))
> +		return;

Isn't this redundant? Because the following has it inside already.

> +	fwnode_for_each_child_node(parent, child)
> +		fwnode_remove_software_node(child);
> +
> +	fwnode_remove_software_node(parent);
> +}

...

>  	const struct soc_button_info *info;
>  	struct platform_device *pd;
> -	struct gpio_keys_button *gpio_keys;
> -	struct gpio_keys_platform_data *gpio_keys_pdata;
> +	struct fwnode_handle *parent_node;

Can it be one line down, so we keep reversed xmas tree order?

>  	const struct dmi_system_id *dmi_id;
>  	int invalid_acpi_index = -1;
> -	int error, gpio, irq = 0;
> +	int error, irq = 0;
>  	int n_buttons = 0;

...

> +		child_props[prop_idx++] = PROPERTY_ENTRY_U32("linux,code", info->event_code);
> +		child_props[prop_idx++] = PROPERTY_ENTRY_STRING("label", info->name);
> +
> +		if (info->event_type != EV_KEY)
> +			child_props[prop_idx++] =

> +				PROPERTY_ENTRY_U32("linux,input-type",
> +						   info->event_type);

Make it a single line, you seem use 100 limit anyway.

> +
> +		if (info->wakeup)
> +			child_props[prop_idx++] = PROPERTY_ENTRY_BOOL("wakeup-source");
> +
> +		/* These devices often use cheap buttons, use 50 ms debounce */
> +		child_props[prop_idx++] = PROPERTY_ENTRY_U32("debounce-interval", 50);

...

>  static void soc_button_remove(struct platform_device *pdev)
>  {
>  	struct soc_button_data *priv = platform_get_drvdata(pdev);
> -
> +	struct platform_device *pd;
> +	struct fwnode_handle *fwnode;
>  	int i;
>  
> -	for (i = 0; i < BUTTON_TYPES; i++)
> -		if (priv->children[i])
> -			platform_device_unregister(priv->children[i]);
> +	for (i = 0; i < BUTTON_TYPES; i++) {

At the same time this can be transformed to

	for (int i = 0; i < BUTTON_TYPES; i++) {

> +		pd = priv->children[i];
> +		if (pd) {

		if (!pd)
			continue;

?

> +			fwnode = dev_fwnode(&pd->dev);
> +			platform_device_unregister(pd);
> +			soc_button_remove_swnodes(fwnode);
> +		}
> +	}
>  }

-- 
With Best Regards,
Andy Shevchenko