Upstream work for OpenFSI patches

Jeremy Kerr jk at ozlabs.org
Fri Mar 24 13:37:11 AEDT 2017


Hi Chris,

Thanks for taking a look.

> Have you run much testing on bad bus accesses to verify recovery from
> errors?

Yes, I've tested two main scenarios:

 1) read from the SBE FIFO (CFAM addr 0xc00) when no FIFO data is present

    this results in endless BUSY/DPOLL, which we now correctly recover
    entirely within the fsi_master_gpio_xfer() function, as it now
    supports sending TERM

 2) accesses to non-existent slaves behind a cMFSI (CFAM addr 0x80000,
    referencing an unused cMFSI link)

    this is handled by the core error handler, which recovers the CFAM
    non-intrusively

If there's other areas that need testing, let me know.

>>   - Hub master: this is now implemented as a fsi engine driver, as the
>>     fsi_slave_{read,write}() functions are exported (and the port count
>>     is available in the hMFSI configuration register)
> I noticed you moved the FSI master control register definitions into
> fsi-master-hub.c   Though its not presently used, the cmFSI engine also
> shares a common control register setup for the most part and its
> possible there could be other implementations using the same scheme in
> the future.   Would it be preferable to move these into fsi-master.h or
> some other common master code?

We should move it to common code when (or if) we need it there. It may
be that we end up handling both cMFSI and hMFSI with the same driver.

I think it's a good idea to keep the scope of things as small as
possible, until usage dictates otherwise.

>>   - IRQ support needs more work, but there doesn't seem to be any
>>     consumers of that so far
> As long as we migrate to the final IIC driver solution before switching
> over to your new code then there is no interrupt support required. 
> Otherwise we'll need to consider when your changes are phased in.

OK. The IRQ support needs a bit of a rework, and we will probably want
to use the generic Linux irq infrastructure to implement that. We'll
probably also need it for whatever the final i2c driver implementation
is, so I'll bump that up my priority list.

>> So, please take a look at the series and let me know if there is
>> anything that still needs doing (particularly fixes that have come in
>> since the first series), and we can work out our plan for upstream
>> progression.
> The community had wanted crc calculations moved from drivers/fsi to the
> kernel/lib path on a previous patch set submission I sent out. Do you
> think this is something that will be required in your changes?

The last email I'd sent about that (to Greg) was asking how we would
handle different polynomial values if we were to move it to lib/. I
didn't get an answer to that, so I've added this to the commit message:

    We add this to the fsi code (rather than lib/), as we need a specific
    polynomial for FSI CRCs.

So, if we receive the same feedback, we'll need to figure out how to
handle that.

> The community also didn't want the fake FSI master driver included.   Is
> this something we should keep?

Was that feedback from me, or upstream?

Regardless, I've marked it as DEBUG, so am not intending to send it :)

Cheers,


Jeremy


More information about the openbmc mailing list