Re: [PATCH v5 4/7] drm/xe/i2c: use platform_device_add_software_node()

["Danilo Krummrich" <dakr@kernel.org>]

On Tue May 12, 2026 at 1:59 PM CEST, Bartosz Golaszewski wrote:
> diff --git a/drivers/gpu/drm/xe/xe_i2c.c b/drivers/gpu/drm/xe/xe_i2c.c
> index 706783863d07d66b4685005d6649b3cd143ecc3b..7f4295e7dc74f112abff8427485b5c8a5ad71383 100644
> --- a/drivers/gpu/drm/xe/xe_i2c.c
> +++ b/drivers/gpu/drm/xe/xe_i2c.c
> @@ -53,6 +53,10 @@ static const struct property_entry xe_i2c_adapter_properties[] = {
>  	{ }
>  };
>  
> +static const struct software_node xe_i2c_adapter_swnode = {
> +	.properties = xe_i2c_adapter_properties,
> +};
> +
>  static inline void xe_i2c_read_endpoint(struct xe_mmio *mmio, void *ep)
>  {
>  	u32 *val = ep;
> @@ -96,10 +100,6 @@ static int xe_i2c_register_adapter(struct xe_i2c *i2c)
>  	struct fwnode_handle *fwnode;
>  	int ret;
>  
> -	fwnode = fwnode_create_software_node(xe_i2c_adapter_properties, NULL);
> -	if (IS_ERR(fwnode))
> -		return PTR_ERR(fwnode);
> -
>  	/*
>  	 * Not using platform_device_register_full() here because we don't have
>  	 * a handle to the platform_device before it returns. xe_i2c_notifier()
> @@ -107,10 +107,12 @@ static int xe_i2c_register_adapter(struct xe_i2c *i2c)
>  	 * platform_device_register_full() is done.
>  	 */
>  	pdev = platform_device_alloc(adapter_name, pci_dev_id(pci));
> -	if (!pdev) {
> -		ret = -ENOMEM;
> -		goto err_fwnode_remove;
> -	}
> +	if (!pdev)
> +		return -ENOMEM;
> +
> +	ret = platform_device_add_software_node(pdev, &xe_i2c_adapter_swnode);

By calling platform_device_add_software_node() the platform device technically
takes ownership of the struct software_node managing its lifetime, by removing
it in release().

However, this is not properly reflected by the
platform_device_add_software_node() API, as it just stores the pointer given by
the driver, which in this case is a pointer to module memory.

The platform device is reference counted and can outlive the module.

So, I think analogous to platform_device_add_data() we need to make a copy of
the struct software_node in platform_device_add_software_node().

> +	if (ret)
> +		goto err_pdev_put;
>  
>  	if (i2c->adapter_irq) {
>  		struct resource res;
> @@ -123,7 +125,6 @@ static int xe_i2c_register_adapter(struct xe_i2c *i2c)
>  	}
>  
>  	pdev->dev.parent = i2c->drm_dev;
> -	pdev->dev.fwnode = fwnode;
>  	i2c->adapter_node = fwnode;
>  	i2c->pdev = pdev;
>  
> @@ -135,8 +136,6 @@ static int xe_i2c_register_adapter(struct xe_i2c *i2c)
>  
>  err_pdev_put:
>  	platform_device_put(pdev);
> -err_fwnode_remove:
> -	fwnode_remove_software_node(fwnode);

There is another call to this in  xe_i2c_unregister_adapter(), which I think was
missed.