[PATCH] scsi: ufs: core: Add a vop to handle vendor specific ops

[PATCH] scsi: ufs: core: Add a vop to handle vendor specific ops

[Hongjie Fang]

add a vop to allow some vendors to do some additional ops
for some interrupts if necessary.

Signed-off-by: Hongjie Fang <hongjiefang@asrmicro.com>
---
 drivers/ufs/core/ufshcd-priv.h | 6 ++++++
 drivers/ufs/core/ufshcd.c      | 2 ++
 include/ufs/ufshcd.h           | 2 ++
 3 files changed, 10 insertions(+)

diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h
index 37c32071e754..1d3dcbc2fda3 100644
--- a/drivers/ufs/core/ufshcd-priv.h
+++ b/drivers/ufs/core/ufshcd-priv.h
@@ -287,6 +287,12 @@ static inline u32 ufshcd_vops_freq_to_gear_speed(struct ufs_hba *hba, unsigned l
 	return 0;
 }
 
+static inline void ufshcd_vops_vendor_intr(struct ufs_hba *hba)
+{
+	if (hba->vops && hba->vops->vendor_intr)
+		hba->vops->vendor_intr(hba);
+}
+
 extern const struct ufs_pm_lvl_states ufs_pm_lvl_states[];
 
 /**
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 9ceb6d6d479d..be51418e09c2 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -7141,6 +7141,8 @@ static irqreturn_t ufshcd_sl_intr(struct ufs_hba *hba, u32 intr_status)
 {
 	irqreturn_t retval = IRQ_NONE;
 
+	ufshcd_vops_vendor_intr(hba);
+
 	if (intr_status & UFSHCD_UIC_MASK)
 		retval |= ufshcd_uic_cmd_compl(hba, intr_status);
 
diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h
index 8563b6648976..a8eca8c22e7f 100644
--- a/include/ufs/ufshcd.h
+++ b/include/ufs/ufshcd.h
@@ -331,6 +331,7 @@ struct ufs_pwr_mode_info {
  * @config_esi: called to config Event Specific Interrupt
  * @config_scsi_dev: called to configure SCSI device parameters
  * @freq_to_gear_speed: called to map clock frequency to the max supported gear speed
+ * @vendor_intr: called before the interrupts handle
  */
 struct ufs_hba_variant_ops {
 	const char *name;
@@ -380,6 +381,7 @@ struct ufs_hba_variant_ops {
 	int	(*config_esi)(struct ufs_hba *hba);
 	void	(*config_scsi_dev)(struct scsi_device *sdev);
 	u32	(*freq_to_gear_speed)(struct ufs_hba *hba, unsigned long freq);
+	void    (*vendor_intr)(struct ufs_hba *hba);
 };
 
 /* clock gating state  */
-- 
2.25.1

Re: [PATCH] scsi: ufs: core: Add a vop to handle vendor specific ops

[Bart Van Assche]

On 3/19/26 2:38 AM, Hongjie Fang wrote:
> add a vop to allow some vendors to do some additional ops
> for some interrupts if necessary.

UFS patches should be sent to Martin K. Petersen and should be Cc-ed to
the linux-scsi mailing list. Additionally, a patch description should
not only explain what has been changed but also why a change is being
mode. "to do some additional ops for some interrupts if necessary" is
too vague.

> @@ -7141,6 +7141,8 @@ static irqreturn_t ufshcd_sl_intr(struct ufs_hba *hba, u32 intr_status)
>   {
>   	irqreturn_t retval = IRQ_NONE;
>   
> +	ufshcd_vops_vendor_intr(hba);
Why to call this code from inside ufshcd_sl_intr() instead of from the 
ufshcd_sl_intr() caller?

> @@ -380,6 +381,7 @@ struct ufs_hba_variant_ops {
>   	int	(*config_esi)(struct ufs_hba *hba);
>   	void	(*config_scsi_dev)(struct scsi_device *sdev);
>   	u32	(*freq_to_gear_speed)(struct ufs_hba *hba, unsigned long freq);
> +	void    (*vendor_intr)(struct ufs_hba *hba);
>   };

Where is the implementation of .vendor_intr? I don't see any 
implementation of that new callback in this patch. Please always submit
at least one implementation of a new vendor operation together with the
patch that adds the new vendor operation.

Thanks,

Bart.

RE: [PATCH] scsi: ufs: core: Add a vop to handle vendor specific ops

[Fang Hongjie(方洪杰)]

> On 3/19/26 2:38 AM, Hongjie Fang wrote:
> > add a vop to allow some vendors to do some additional ops
> > for some interrupts if necessary.
> 
> UFS patches should be sent to Martin K. Petersen and should be Cc-ed to
> the linux-scsi mailing list. Additionally, a patch description should
> not only explain what has been changed but also why a change is being
> mode. "to do some additional ops for some interrupts if necessary" is
> too vague.

Given that some UFS controllers have private or extended interrupt status 
registers, the purpose of this patch is to facilitate the handling of 
proprietary registers within the host driver during the interrupt handling.

> 
> > @@ -7141,6 +7141,8 @@ static irqreturn_t ufshcd_sl_intr(struct ufs_hba
> *hba, u32 intr_status)
> >   {
> >   	irqreturn_t retval = IRQ_NONE;
> >
> > +	ufshcd_vops_vendor_intr(hba);
> Why to call this code from inside ufshcd_sl_intr() instead of from the
> ufshcd_sl_intr() caller?

It is called within `ufshcd_sl_intr()` to support running vendor ops in 
the `ufshcd_threaded_intr()`.

> 
> > @@ -380,6 +381,7 @@ struct ufs_hba_variant_ops {
> >   	int	(*config_esi)(struct ufs_hba *hba);
> >   	void	(*config_scsi_dev)(struct scsi_device *sdev);
> >   	u32	(*freq_to_gear_speed)(struct ufs_hba *hba, unsigned long
> freq);
> > +	void    (*vendor_intr)(struct ufs_hba *hba);
> >   };
> 
> Where is the implementation of .vendor_intr? I don't see any
> implementation of that new callback in this patch. Please always submit
> at least one implementation of a new vendor operation together with the
> patch that adds the new vendor operation.

The idea here is to first enable the ability to run vendor operations within
interrupt handlers, making it easier to add the vendor specific
implementations later.

> 
> Thanks,
> 
> Bart.

Best.

Re: [PATCH] scsi: ufs: core: Add a vop to handle vendor specific ops

[Bart Van Assche]

On 3/20/26 8:34 PM, Fang Hongjie(方洪杰) wrote:
>> On 3/19/26 2:38 AM, Hongjie Fang wrote:
>>> add a vop to allow some vendors to do some additional ops
>>> for some interrupts if necessary.
>>
>> UFS patches should be sent to Martin K. Petersen and should be Cc-ed to
>> the linux-scsi mailing list. Additionally, a patch description should
>> not only explain what has been changed but also why a change is being
>> mode. "to do some additional ops for some interrupts if necessary" is
>> too vague.
> 
> Given that some UFS controllers have private or extended interrupt status
> registers, the purpose of this patch is to facilitate the handling of
> proprietary registers within the host driver during the interrupt handling.

The above makes it clear that this patch is intended for a UFS host
controller that does not comply to the JEDEC UFSHCI standard. The Linux
kernel is standards based and the upstream Linux kernel UFS
driver is for UFS host controllers that comply to the JEDEC UFSHCI
standard.

Thanks,

Bart.