Re: [PATCH 2/2] ufs: core: Really fix the code for updating the "hid" attribute group

[Bart Van Assche <bvanassche@acm.org>]

On 10/27/25 8:03 PM, Bjorn Andersson wrote:
> So, unless I'm missing something with regards to the error handler that
> you refer to, I think you should solve this problem in ufshcd_init(), by
> making sure that syfs attributes are created before the
> ufshcd_device_params_init() call.

Something like this? (needs more work to make sure all state information 
that can be modified through sysfs has been initialized before the sysfs 
attributes are added)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 5d6297aa5c28..3ad258922036 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -10800,6 +10800,8 @@ int ufshcd_init(struct ufs_hba *hba, void 
__iomem *mmio_base, unsigned int irq)
          */
         ufshcd_readl(hba, REG_INTERRUPT_ENABLE);

+       ufs_sysfs_add_nodes(hba->dev);
+
         /* IRQ registration */
         err = devm_request_threaded_irq(dev, irq, ufshcd_intr, 
ufshcd_threaded_intr,
                                         IRQF_ONESHOT | IRQF_SHARED, 
UFSHCD, hba);
@@ -10887,7 +10889,6 @@ int ufshcd_init(struct ufs_hba *hba, void 
__iomem *mmio_base, unsigned int irq)
         if (err)
                 goto out_disable;

-       ufs_sysfs_add_nodes(hba->dev);
         async_schedule(ufshcd_async_scan, hba);

         device_enable_async_suspend(dev);

> PS. How come these are attributes on the host device and not on the SCSI
> host device (i.e. ufshcd_driver_template::shost_groups)? It seems like
> more structured place to have such properties, and would avoid having to
> dynamically create/destroy them from the ufshcd driver itself.

This choice was made before I started contributing to the UFSHCI driver.
I'm not sure why this choice has been made. If we would start over from
scratch, I think the UFSHCI attributes should be added to the UFSHCI
device instance and the UFS attributes should be added to the SCSI host.
Today the sysfs attributes created by the UFSHCI driver are a mix of
UFSHCI and UFS attributes.

This is a change we can't make today because it would break user space.

Thanks,

Bart.