[PATCH linux 14/15] drivers/fsi: Rename slave to cfam

[Christopher Bostic]

On Thu, Oct 6, 2016 at 8:22 PM, Jeremy Kerr <jk at ozlabs.org> wrote:
> Hi Chris,
>
>>>> Suggested changes to the patch set so far as provided by Jeremy Kerr.
>>>> Rename slave to cfam to distinguish between the slave engine and its
>>>> containing cfam.
>>>
>>> I'm still not 100% on that. As is my understanding, a (hardware) CFAM
>>> contains multiple slaves, and that not what `struct fsi_slave`
>>> represents (it's a single slave).
>>
>> In redundant configurations yes there are two slaves.  However, any
>> controlling device driver would only ever use one of them.  A redundant
>> system with its own fsi device driver would be controlling the other slave.
>>
>> I still maintain that there is bound to be confusion with slave->read( )
>> etc... which are meant for accessing more than just the slave engine.
>> cfam->read makes it more clear I think that you are accessing some
>> range within the cfam, not just the slave engine.
>>
>> A cfam acts as a slave
>> in a general sense but it doesn't distinguish itself from the slave engine.
>
> Right, and that's where our confusion is coming from there.
>
> In my initial series, I've used `struct fsi_slave` to represent *just*
> the slave on the FSI bus, which is logically separated from the *slave
> engines*. In this case, a "slave" is a different logical unit from a
> "slave engine"; the latter being one of the actual functional units made
> available behind the slave.
>
> This separation is important, as each of the engines is what will be
> bound to a FSI device driver, whereas the slave itself will be managed
> by the core.
> So we have:
>
>   struct fsi_slave -> core slave logic, knows little about engines
>
>   struct fsi_device -> engine logic, to be bound to a driver
>
> (whereas a CFAM is slave + engines, right?)
>
> Nothing outside of the core will be doing slave->read; FSI engine
> drivers will be using the fsi_device API.

Hi Jeremy,

That makes sense.  I wasn't getting what you were thinking.  I'll
keep your 'struct fsi_slave' as is.   The previous note regarding
hardware workarounds, though, will possibly require some concept
of a separate 'struct cfam'.   That or some other construct to
deal in a common way with all the quirks related to a particular
cfam.  Can you make Chris Austen's call Tuesday morning
your time?  We can discuss that further.

>
>> I'm not sure what split out means in this case.  Git am applies each patch
>> in sequence so those warnings will be hit before any patches I add to reverse
>> something are applied.
>>
>> I know we had discussed last week about making it clear what changes
>> you had made versus mine.  As per discussion I left your changes as
>> is in the patch series, then made my recommended changes as the
>> next in the series.   A bit confusing on how to deal with checkpatch,
>> git am, ... in those circumstances.
>
> Yep, and if there are fixups to those patches, keep them separate from
> other changes, as I'll apply them directly to my tree.

So the plan is,  patches 01-12 your set.  patch 13 are my fixups to that.
Patches 14+ my content.

>
> From your follow-up email, it sounds like we want some identification of
> the slave HW implementation, to allow the core and drivers to work
> around any HW issues.
>
> Cheers,
>
>
> Jeremy