Re: [PATCH 1/2] i3c: Add support for HDR modes.

[vitor <vitor.soares@synopsys.com>]

On 25/02/19 13:10, Boris Brezillon wrote:
>>>>>> something more generic than what we currently have.
>>>>>>
>>>>>> Something like
>>>>>>
>>>>>> enum i3c_xfer_type {
>>>>>> 	I3C_CCC_XFER,
>>>>>> 	I3C_SDR_XFER,
>>>>>> 	I3C_HDR_XFER,
>>>>>> }
>>>>>>
>>>>>> struct i3c_xfer {
>>>>>> 	enum i3c_xfer_type type;
>>>>>> 	union {
>>>>>> 		struct i3c_ccc_cmd ccc;
>>>>>> 		struct i3c_priv_xfer sdr;
>>>>>> 		struct i3c_hdr_cmd hdr;
>>>>>> 	};
>>>>>> }    
>>>> @Boris: I remember you don't want to expose CCC commands, I think at least for now we shouldn't expose too.
>>>>
>>>> Do you agree?  
>>> Well, if we want to have a generic interface and not force all drivers
>>> to implement one hook per transfer type, we should also have CCC
>>> commands in this enum. We can still make sure the user does not pass
>>> CCC commands before forwarding the request to the controller.  
>> I'm ok with this. Still there is some CCC commands that might be expose for devices and we can decide it later.
>>
>>> Anyway, as for all the other changes proposed so far, I'd like to see a
>>> real use case for this SDR/HDR[/CCC] mix. Do you have a device spec
>>> describing such a sequence?  
>> From my knowledge there is no devices HDR capable on the market thus I cannot argue with real use case.
>>
>> But per i3c spec v1.0 there is no mention that HDR frame shall begin with a START condition (not REPEATED START).
>> So, the question is why do not support it instead of limiting it?
> Well, all depends on the extra complexity to support this case. If it's
> simple enough, I won't complain.
>
>> We don't know the implementation of the device and some of them maybe need the previous transfer (in SDR) to do the next one (in HDR).
> Exactly, we don't know, so maybe we should just wait before introducing
> HDR mode...
>
>> and supporting it we are cover more use cases.
> Are we even sure all controllers will be able to chain things like
> that? 

Why not? for the controllers shouldn't be a problem. The problem can be if it doesn't support.

> For instance, in it's non-DMA mode, the Cadence controller is
> limited by the FIFO size, which means you anyway won't be able to
> chain too much frames using a REPEATED START. So, any device relying on
> REPEATED START for atomic sequences is unlikely to work with all master
> controllers.

IMO this isn't up to us to decide, we just need to provide support and try to cover the most possible use cases.
Right now we only have 2 masters to support this kind of change and I just see some effort to support the CCC commands.

I would like to test this scenario, what do you think that can add extra complexity?


Best regards,
Vitor Soares

_______________________________________________
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c