Re: [PATCH v1 2/5] dmaengine: ioatdma: use common channel sysfs attribute creation

[Dave Jiang <dave.jiang@intel.com>]

On 7/7/26 9:20 AM, Logan Gunthorpe wrote:
> Instead of manually creating and adding sysfs attributes to each channel
> use the common dma_chan_kobject_add() facility.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>

Acked-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/dma/ioat/dma.h   |  2 -
>  drivers/dma/ioat/init.c  |  4 +-
>  drivers/dma/ioat/sysfs.c | 88 +++-------------------------------------
>  3 files changed, 7 insertions(+), 87 deletions(-)
> 
> diff --git a/drivers/dma/ioat/dma.h b/drivers/dma/ioat/dma.h
> index e8a880f338c6..bea2a0101ede 100644
> --- a/drivers/dma/ioat/dma.h
> +++ b/drivers/dma/ioat/dma.h
> @@ -393,8 +393,6 @@ void ioat_issue_pending(struct dma_chan *chan);
>  /* IOAT Init functions */
>  bool is_bwd_ioat(struct pci_dev *pdev);
>  struct dca_provider *ioat_dca_init(struct pci_dev *pdev, void __iomem *iobase);
> -void ioat_kobject_add(struct ioatdma_device *ioat_dma, const struct kobj_type *type);
> -void ioat_kobject_del(struct ioatdma_device *ioat_dma);
>  int ioat_dma_setup_interrupts(struct ioatdma_device *ioat_dma);
>  void ioat_stop(struct ioatdma_chan *ioat_chan);
>  #endif /* IOATDMA_H */
> diff --git a/drivers/dma/ioat/init.c b/drivers/dma/ioat/init.c
> index 737496391109..82f76fd0ccc1 100644
> --- a/drivers/dma/ioat/init.c
> +++ b/drivers/dma/ioat/init.c
> @@ -541,8 +541,6 @@ static void ioat_dma_remove(struct ioatdma_device *ioat_dma)
>  
>  	ioat_disable_interrupts(ioat_dma);
>  
> -	ioat_kobject_del(ioat_dma);
> -
>  	dma_async_device_unregister(dma);
>  }
>  
> @@ -1174,7 +1172,7 @@ static int ioat3_dma_probe(struct ioatdma_device *ioat_dma, int dca)
>  	if (err)
>  		goto err_disable_interrupts;
>  
> -	ioat_kobject_add(ioat_dma, &ioat_ktype);
> +	dma_chan_kobject_add(&ioat_dma->dma_dev, &ioat_ktype, "quickdata");
>  
>  	if (dca)
>  		ioat_dma->dca = ioat_dca_init(pdev, ioat_dma->reg_base);
> diff --git a/drivers/dma/ioat/sysfs.c b/drivers/dma/ioat/sysfs.c
> index e796ddb5383f..4dc61fdddb21 100644
> --- a/drivers/dma/ioat/sysfs.c
> +++ b/drivers/dma/ioat/sysfs.c
> @@ -14,12 +14,6 @@
>  
>  #include "../dmaengine.h"
>  
> -struct ioat_sysfs_entry {
> -	struct attribute attr;
> -	ssize_t (*show)(struct dma_chan *, char *);
> -	ssize_t (*store)(struct dma_chan *, const char *, size_t);
> -};
> -
>  static ssize_t cap_show(struct dma_chan *c, char *page)
>  {
>  	struct dma_device *dma = c->device;
> @@ -32,7 +26,7 @@ static ssize_t cap_show(struct dma_chan *c, char *page)
>  		       dma_has_cap(DMA_INTERRUPT, dma->cap_mask) ? " intr" : "");
>  
>  }
> -static const struct ioat_sysfs_entry ioat_cap_attr = __ATTR_RO(cap);
> +static const struct dma_chan_sysfs_entry ioat_cap_attr = __ATTR_RO(cap);
>  
>  static ssize_t version_show(struct dma_chan *c, char *page)
>  {
> @@ -42,77 +36,7 @@ static ssize_t version_show(struct dma_chan *c, char *page)
>  	return sprintf(page, "%d.%d\n",
>  		       ioat_dma->version >> 4, ioat_dma->version & 0xf);
>  }
> -static const struct ioat_sysfs_entry ioat_version_attr = __ATTR_RO(version);
> -
> -static ssize_t
> -ioat_attr_show(struct kobject *kobj, struct attribute *attr, char *page)
> -{
> -	const struct ioat_sysfs_entry *entry;
> -	struct ioatdma_chan *ioat_chan;
> -
> -	entry = container_of_const(attr, struct ioat_sysfs_entry, attr);
> -	ioat_chan = container_of(kobj, struct ioatdma_chan, kobj);
> -
> -	if (!entry->show)
> -		return -EIO;
> -	return entry->show(&ioat_chan->dma_chan, page);
> -}
> -
> -static ssize_t
> -ioat_attr_store(struct kobject *kobj, struct attribute *attr,
> -const char *page, size_t count)
> -{
> -	const struct ioat_sysfs_entry *entry;
> -	struct ioatdma_chan *ioat_chan;
> -
> -	entry = container_of_const(attr, struct ioat_sysfs_entry, attr);
> -	ioat_chan = container_of(kobj, struct ioatdma_chan, kobj);
> -
> -	if (!entry->store)
> -		return -EIO;
> -	return entry->store(&ioat_chan->dma_chan, page, count);
> -}
> -
> -static const struct sysfs_ops ioat_sysfs_ops = {
> -	.show	= ioat_attr_show,
> -	.store  = ioat_attr_store,
> -};
> -
> -void ioat_kobject_add(struct ioatdma_device *ioat_dma, const struct kobj_type *type)
> -{
> -	struct dma_device *dma = &ioat_dma->dma_dev;
> -	struct dma_chan *c;
> -
> -	list_for_each_entry(c, &dma->channels, device_node) {
> -		struct ioatdma_chan *ioat_chan = to_ioat_chan(c);
> -		struct kobject *parent = &c->dev->device.kobj;
> -		int err;
> -
> -		err = kobject_init_and_add(&ioat_chan->kobj, type,
> -					   parent, "quickdata");
> -		if (err) {
> -			dev_warn(to_dev(ioat_chan),
> -				 "sysfs init error (%d), continuing...\n", err);
> -			kobject_put(&ioat_chan->kobj);
> -			set_bit(IOAT_KOBJ_INIT_FAIL, &ioat_chan->state);
> -		}
> -	}
> -}
> -
> -void ioat_kobject_del(struct ioatdma_device *ioat_dma)
> -{
> -	struct dma_device *dma = &ioat_dma->dma_dev;
> -	struct dma_chan *c;
> -
> -	list_for_each_entry(c, &dma->channels, device_node) {
> -		struct ioatdma_chan *ioat_chan = to_ioat_chan(c);
> -
> -		if (!test_bit(IOAT_KOBJ_INIT_FAIL, &ioat_chan->state)) {
> -			kobject_del(&ioat_chan->kobj);
> -			kobject_put(&ioat_chan->kobj);
> -		}
> -	}
> -}
> +static const struct dma_chan_sysfs_entry ioat_version_attr = __ATTR_RO(version);
>  
>  static ssize_t ring_size_show(struct dma_chan *c, char *page)
>  {
> @@ -120,7 +44,7 @@ static ssize_t ring_size_show(struct dma_chan *c, char *page)
>  
>  	return sprintf(page, "%d\n", (1 << ioat_chan->alloc_order) & ~1);
>  }
> -static const struct ioat_sysfs_entry ring_size_attr = __ATTR_RO(ring_size);
> +static const struct dma_chan_sysfs_entry ring_size_attr = __ATTR_RO(ring_size);
>  
>  static ssize_t ring_active_show(struct dma_chan *c, char *page)
>  {
> @@ -129,7 +53,7 @@ static ssize_t ring_active_show(struct dma_chan *c, char *page)
>  	/* ...taken outside the lock, no need to be precise */
>  	return sprintf(page, "%d\n", ioat_ring_active(ioat_chan));
>  }
> -static const struct ioat_sysfs_entry ring_active_attr = __ATTR_RO(ring_active);
> +static const struct dma_chan_sysfs_entry ring_active_attr = __ATTR_RO(ring_active);
>  
>  static ssize_t intr_coalesce_show(struct dma_chan *c, char *page)
>  {
> @@ -154,7 +78,7 @@ size_t count)
>  	return count;
>  }
>  
> -static const struct ioat_sysfs_entry intr_coalesce_attr = __ATTR_RW(intr_coalesce);
> +static const struct dma_chan_sysfs_entry intr_coalesce_attr = __ATTR_RW(intr_coalesce);
>  
>  static const struct attribute *const ioat_attrs[] = {
>  	&ring_size_attr.attr,
> @@ -167,6 +91,6 @@ static const struct attribute *const ioat_attrs[] = {
>  ATTRIBUTE_GROUPS(ioat);
>  
>  const struct kobj_type ioat_ktype = {
> -	.sysfs_ops = &ioat_sysfs_ops,
> +	.sysfs_ops = &dma_chan_sysfs_ops,
>  	.default_groups = ioat_groups,
>  };