Re: [PATCH] bus: mhi: host: Flush the posted write after writing to MHI_SOC_RESET_REQ_OFFSET

[Jeff Hugo]

On 6/23/2026 8:51 AM, Manivannan Sadhasivam wrote:
> mhi_soc_reset() tries to reset the device by writing to the
> MHI_SOC_RESET_REQ_OFFSET register. But it doesn't do a read-back to ensure
> that the write gets flushed to the device before returning to the caller.
>
> This may lead to the delay (if implemented) on the caller to be
> insufficient, if the posted write doesn't reach the device before the
> delay.

Interesting.  Is the delay tight enough that a few ms will possibly blow 
it?  Seems like a poorly defined delay.  All the devices I'm familiar 
with take multiple seconds to boot (with some variability due to ddr 
training and thermal constraints), and if the reset triggers a crash 
dump, then its easily tens of seconds.

Regardless, since this reset will either kill the pcie link, or 
disconnect the SoC from the link for a time, I've been trying to figure 
out how this change might break, but I haven't found a scenario, so I 
suspect this is good enough.

> So add a read-back after writing to the MHI_SOC_RESET_REQ_OFFSET register.
>
> Fixes: b5a8d233a588 ("bus: mhi: core: Add device hardware reset support")
> Reported-by: Alex Williamson <alex@xxxxxxxxxxx>
> Closes: https://lore.kernel.org/linux-pci/20260622160822.09350246@xxxxxxxxxxx
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxxxxxxxx>
> ---
>   drivers/bus/mhi/host/main.c | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c
> index 53c0ffe30070..4d458396233a 100644
> --- a/drivers/bus/mhi/host/main.c
> +++ b/drivers/bus/mhi/host/main.c
> @@ -170,6 +170,9 @@ EXPORT_SYMBOL_GPL(mhi_get_mhi_state);
>   
>   void mhi_soc_reset(struct mhi_controller *mhi_cntrl)
>   {
> +	int __maybe_unused ret;
> +	u32 tmp;
> +
>   	if (mhi_cntrl->reset) {
>   		mhi_cntrl->reset(mhi_cntrl);
>   		return;
> @@ -178,6 +181,9 @@ void mhi_soc_reset(struct mhi_controller *mhi_cntrl)
>   	/* Generic MHI SoC reset */
>   	mhi_write_reg(mhi_cntrl, mhi_cntrl->regs, MHI_SOC_RESET_REQ_OFFSET,
>   		      MHI_SOC_RESET_REQ);
> +	/* Flush the posted write to the device (ignore return value) */
> +	ret = mhi_read_reg(mhi_cntrl, mhi_cntrl->regs, MHI_SOC_RESET_REQ_OFFSET,
> +			   &tmp);

If you wanted, you could fit this all on one line.  word wrapping for a 
3 char parameter seems a bit silly to me, but I suspect this is highly 
subjective.

Reviewed-by: Jeff Hugo <jeff.hugo@xxxxxxxxxxxxxxxx>