Re: [PATCH 01/10] ufs: unipro: Move Tx/Rx FSM state definitions to unipro.h

[Larisa Ileana Grigore]

On 8/26/2026 6:05 PM, Frank Li wrote:
> On Wed, Aug 26, 2026 at 03:40:39PM +0200, Larisa Grigore wrote:
> > [You don't often get email from larisa.grigore@xxxxxxxxxxx. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> >
> > Move the Tx/Rx FSM state definitions from ufshcd-dwc.h to
> > include/ufs/unipro.h.
> >
> > These state values are UniPro/M-PHY related and are not specific to
> > the DesignWare UFS host controller implementation, so unipro.h is a
> > more appropriate location for sharing them.
> >
> > As part of this move, drop the driver-private TX_FSM_HIBERN8 define in
> > ufs-hisi and ufs-qcom and repoint their Hibern8 checks at the shared
> > TX_STATE_HIBERN8 enum constant. This is a no-op change: TX_FSM_HIBERN8
> > was 0x1 and TX_STATE_HIBERN8 is 1, so the compared values are identical.
> >
> > This also prepares for using these definitions in a subsequent commit.
> >
> > Signed-off-by: Larisa Grigore <larisa.grigore@xxxxxxxxxxx>
> > ---
> ...
> > -enum rx_fsm_state {
> > -       RX_STATE_DISABLED = 0,
> > -       RX_STATE_HIBERN8 = 1,
> > -       RX_STATE_SLEEP = 2,
> > -       RX_STATE_STALL = 3,
> > -       RX_STATE_LSBURST = 4,
> > -       RX_STATE_HSBURST = 5,
> > -};
> > -
> > -enum tx_fsm_state {
> > -       TX_STATE_DISABLED = 0,
> > -       TX_STATE_HIBERN8 = 1,
> > -       TX_STATE_SLEEP = 2,
> > -       TX_STATE_STALL = 3,
> > -       TX_STATE_LSBURST = 4,
> > -       TX_STATE_HSBURST = 5,
> > -};
> > -
> >   struct ufshcd_dme_attr_val {
> >          u32 attr_sel;
> >          u32 mib_val;
> > diff --git a/include/ufs/unipro.h b/include/ufs/unipro.h
> > index 9c168703b104..bf063edc284d 100644
> > --- a/include/ufs/unipro.h
> > +++ b/include/ufs/unipro.h
> > @@ -466,4 +466,23 @@ enum {
> >          CPORT_CONNECTED,
> >   };
> >
> > +/* Tx/Rx FSM state */
> > +enum ufs_rx_fsm_state {
> > +       RX_STATE_DISABLED = 0,
> > +       RX_STATE_HIBERN8 = 1,
> > +       RX_STATE_SLEEP = 2,
> > +       RX_STATE_STALL = 3,
> > +       RX_STATE_LSBURST = 4,
> > +       RX_STATE_HSBURST = 5,
> > +};
> > +
> > +enum ufs_tx_fsm_state {
> > +       TX_STATE_DISABLED = 0,
> > +       TX_STATE_HIBERN8 = 1,
> > +       TX_STATE_SLEEP = 2,
> > +       TX_STATE_STALL = 3,
> > +       TX_STATE_LSBURST = 4,
> > +       TX_STATE_HSBURST = 5,
> > +};
>
> Suppose ref RX(TX)_FSM_State in mipi m-phy speci v6.0, table 50 M-RX status
> attributes. you add ref to mipi M-phy to let reader know it is really
> defined by spec, not dwc implementation.
>
> Both value are the same, why need define two copy.
>
> Frank

Hi Frank,

Thank you for taking the time to review this patch series and for the 
feedback!

The reason the two enums currently look duplicated is that they 
represent the encodings of two distinct MIPI M-PHY attributes, 
RX_FSM_STATE and TX_FSM_STATE. The specification defines identical state 
encodings for both RX and TX FSMs, which is why the values are the same.

My intention was to preserve the original approach and minimize the 
scope of the changes in this patch. Since the existing code already 
treats the RX and TX FSM states separately, I kept that structure when 
moving the definitions to the common header.

I will clarify in the commit message that these encodings come from the 
MIPI M-PHY specification and are not DesignWare-specific.

Regarding the separate enums, I can consolidate them into a single FSM 
state enum if you think that would be a better representation, given 
that the state encodings are identical.

Thanks,
 Larisa

> > +
> >   #endif /* _UNIPRO_H_ */
> > --
> > 2.43.0