Re: [PATCH] fsi: aspeed: Reset master errors after CFAM reset

[Eddie James]

On 3/8/23 23:38, Joel Stanley wrote:
> On Mon, 6 Mar 2023 at 22:53, Eddie James <eajames@xxxxxxxxxxxxx> wrote:
> > It has been observed that sometimes the FSI master will return all 0xffs
> > after a CFAM has been taken out of reset, without presenting any error.
> > Resetting the FSI master errors resolves the issue.
> Eddie, you mentioned that Joachim was going to provide input on this
> change. Did you hear from him?


Yes, he mentioned it's possibly an addressing issue, with 
absolute/relative/same-address-again type of access. The FSI master 
doesn't know the slave is reset in this case so it needs to go back to 
absolute addressing. Resetting the port on the master is a good solution 
here, though Joachim was surprised that the BREAK command to the master 
didn't clear out the addressing state.


I collected some additional data that perhaps supports this theory: 
After the CFAM reset, I actually get back the data at CFAM address 0x00 
when I read the previously used address. All other addresses return 
0xff. I dumped the master state but there's nothing interesting, and no 
errors.

# /tmp/fsimaster-aspeed cfam 0x9c
CFAM09c: 00000000
# /tmp/fsimaster-aspeed dump master
MMODE[000]: 70080210
MDLYR[004]: ffff0000
MCRSP0[008]: 00000000
MENP0[010]: 80000000
MLEVP0[018]: 80000000
MREFP0[020]: 00000000
MHPMP0[028]: 00000000
MSIEP0[030]: 00000000
MAESP0[050]: 00000000
MAEB[070]: 00000000
MVER[074]: e0050101
MBSYP0[078]: 00000000
MSTAP0[0d0]: 00000000
MSTAP1[0d4]: 00000000
MSTAP2[0d8]: 00000000
MSTAP3[0dc]: 00000000
MSTAP4[0e0]: 00000000
MSTAP5[0e4]: 00000000
MSTAP6[0e8]: 00000000
MSTAP7[0ec]: 00000000
MESRB0[1d0]: 00000000
MSCSB0[1d4]: 90000001
MATRB0[1d8]: 0040013b
MDTRB0[1dc]: 00000000
MECTRL[2e0]: 0000c000
# echo 1 > /sys/class/fsi-master/fsi0/device/cfam_reset
# /tmp/fsimaster-aspeed cfam 0x9c
CFAM09c: c0020da6
# /tmp/fsimaster-aspeed cfam 0x0
CFAM000: ffffffff
# /tmp/fsimaster-aspeed dump master
MMODE[000]: 70080210
MDLYR[004]: ffff0000
MCRSP0[008]: 00000000
MENP0[010]: 80000000
MLEVP0[018]: 80000000
MREFP0[020]: 00000000
MHPMP0[028]: 00000000
MSIEP0[030]: 00000000
MAESP0[050]: 00000000
MAEB[070]: 00000000
MVER[074]: e0050101
MBSYP0[078]: 00000000
MSTAP0[0d0]: 00000000
MSTAP1[0d4]: 00000000
MSTAP2[0d8]: 00000000
MSTAP3[0dc]: 00000000
MSTAP4[0e0]: 00000000
MSTAP5[0e4]: 00000000
MSTAP6[0e8]: 00000000
MSTAP7[0ec]: 00000000
MESRB0[1d0]: 00000000
MSCSB0[1d4]: b0000001
MATRB0[1d8]: 00400003
MDTRB0[1dc]: ffffffff


> Cheers,
>
> Joel
>
> > Signed-off-by: Eddie James <eajames@xxxxxxxxxxxxx>
> > ---
> >   drivers/fsi/fsi-master-aspeed.c | 2 ++
> >   1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/fsi/fsi-master-aspeed.c b/drivers/fsi/fsi-master-aspeed.c
> > index 7cec1772820d..5eccab175e86 100644
> > --- a/drivers/fsi/fsi-master-aspeed.c
> > +++ b/drivers/fsi/fsi-master-aspeed.c
> > @@ -454,6 +454,8 @@ static ssize_t cfam_reset_store(struct device *dev, struct device_attribute *att
> >          gpiod_set_value(aspeed->cfam_reset_gpio, 1);
> >          usleep_range(900, 1000);
> >          gpiod_set_value(aspeed->cfam_reset_gpio, 0);
> > +       usleep_range(900, 1000);
> Is the delay required? How did you arrive at this time?
>
> > +       opb_writel(aspeed, ctrl_base + FSI_MRESP0, cpu_to_be32(FSI_MRESP_RST_ALL_MASTER));
> >          mutex_unlock(&aspeed->lock);
> >          trace_fsi_master_aspeed_cfam_reset(false);
> >
> > --
> > 2.31.1