[PATCH linux v2 16/17] drivers/fsi: Set up CFAMs for communication

[Jeremy Kerr]

Hi Chris,

> For break and smode callbacks I did what you had done for
> fsi_master_fake_write( ).
> I'm not sure how mine are any different?

These implementations don't do anything, so no there's no point
populating those callbacks.

>>> +static int fsi_master_gpio_set_smode(struct fsi_master *_master, int link,
>>> +                                             uint32_t smode)
>>> +{
>>> +     int rc;
>>> +
>>> +     rc = _master->write(_master, link, 0, FSI_SLAVE_BASE + FSI_SMODE,
>>> +                     &smode, sizeof(smode));
>>> +
>>> +     return rc;
>>> +}
>>> +
>>
>> Won't this always be implemented the same way on all masters (ie, a
>> write to a particular slave register)? Why the need for a
>> master-specific callback here?
> 
> 
> This will be different for various masters based on the hardware
> issues mentioned in previous notes.  For example the cascaded master
> needs to set a different ID in smode.

Is sounds like you're putting slave logic (is, the actual value of
smode) into the master code then.

Keep the abstraction at the right level. If the smode depends on whether
the master is cascaded, then all the master needs to expose is whether
or not it's cascaded. It shouldn't need an entirely new callback to do a
write to a particular (fixed?) slave register.

However, since this isn't really used, I can't tell whether it's the
right thing to do or not. Can you defer changes like this (which are
used to support specific functionality - in this case adding cascades)
until their corresponding consumers are added too? This allows us to
review the changes with their proper context.

It seems like the priority at the moment should be getting a single
master going, with interrupts, then we can look at cascading.

Cheers,


Jeremy