Hi Philippe, On Sun, Jan 10, 2021 at 7:15 PM Philippe Mathieu-Daudé <f4...@amsat.org> wrote: > > On 1/10/21 9:14 AM, Bin Meng wrote: > > From: Bin Meng <bin.m...@windriver.com> > > > > Usually the approach is that the device on the other end of the line > > is going to reset its state anyway, so there's no need to actively > > signal an irq line change during the reset hook. > > > > Move imx_spi_update_irq() out of imx_spi_reset(), to a new function > > imx_spi_hard_reset() that is called when the controller is disabled. > > > > Signed-off-by: Bin Meng <bin.m...@windriver.com> > > > > --- > > > > Changes in v4: > > - adujst the patch 2,3 order > > - rename imx_spi_soft_reset() to imx_spi_hard_reset() to avoid confusion > > > > Changes in v3: > > - new patch: remove imx_spi_update_irq() in imx_spi_reset() > > > > hw/ssi/imx_spi.c | 14 ++++++++++---- > > 1 file changed, 10 insertions(+), 4 deletions(-) > > > > diff --git a/hw/ssi/imx_spi.c b/hw/ssi/imx_spi.c > > index e605049a21..2c4c5ec1b8 100644 > > --- a/hw/ssi/imx_spi.c > > +++ b/hw/ssi/imx_spi.c > > @@ -241,11 +241,16 @@ static void imx_spi_reset(DeviceState *dev) > > imx_spi_rxfifo_reset(s); > > imx_spi_txfifo_reset(s); > > > > - imx_spi_update_irq(s); > > - > > s->burst_length = 0; > > } > > > > +static void imx_spi_hard_reset(IMXSPIState *s) > > +{ > > + imx_spi_reset(DEVICE(s)); > > + > > + imx_spi_update_irq(s); > > +} > > + > > static uint64_t imx_spi_read(void *opaque, hwaddr offset, unsigned size) > > { > > uint32_t value = 0; > > @@ -351,8 +356,9 @@ static void imx_spi_write(void *opaque, hwaddr offset, > > uint64_t value, > > s->regs[ECSPI_CONREG] = value; > > > > if (!imx_spi_is_enabled(s)) { > > - /* device is disabled, so this is a reset */ > > - imx_spi_reset(DEVICE(s)); > > + /* device is disabled, so this is a hard reset */ > > + imx_spi_hard_reset(s); > > + > > return; > > } > > > > Almost good :) > > DeviceReset handler is a hard reset, so you need: > > dc->reset = imx_spi_hard_reset; > > Thus you also need this function prototype: > > void imx_spi_hard_reset(DeviceState *dev) > > Eventually renaming imx_spi_reset() -> imx_spi_soft_reset() > will make things even easier.
Now I am confused. The v3 patch did the above already, but you mentioned that usually the hard reset includes the soft reset. But in the hard reset the imx_spi_update_irq() does not need to be called. Regards, Bin