Re: [PATCH 11/16] spi: bcm63xx-hsspi: Add prepend feature support

[William Zhang <william.zhang@broadcom.com>]

[-- Attachment #1: Type: text/plain, Size: 2719 bytes --]



On 01/10/2023 01:18 PM, Mark Brown wrote:
> On Mon, Jan 09, 2023 at 12:43:53PM -0800, William Zhang wrote:
>> On 01/09/2023 11:31 AM, Mark Brown wrote:
> 
>>> If this relies on software control of the chip select (which is what I
>>> *think* your dummy CS workaround thing is about, the other patch about
>>> that is really hard to understand) then I'm confused about what the
>>> advantage is?
> 
>> Dummy CS workaround is implemented by Jonas when he first upstream the
>> driver. It does not work on all the board designs so prepend mode is
>> introduced. I have some detail explanation on this in [PATCH 10/16] spi:
>> bcm63xx-hsspi: Make dummy cs workaround as an option.
> 
> Yes, it is the description in patch 10 that I was having a lot of
> trouble following.
> 
Sorry that my description is not clear...  I can certainly improve it if 
you can let me know what is not clear.

>> The controller only work in one mode and that's why driver code has some
>> dependency between these two modes. The advantage of the premode is it works
>> on all hw design however it does not support all types mem_ops operation.
>> That is why you see the patch 14 to disable the dual io mem op. But dummy cs
>> workaround can support this and in case there is such pattern from non mem
>> op spi transaction, dummy cs workaround can be used as long as it does not
>> have the board design limitation.   So neither one is perfect but hopefully
>> with both options available, we can cover all the cases.
> 
> We can't switch modes per message?
> 
Technically yes. If the code finds the message is not prependable,  it 
can try to use dummy cs workaround to transfer the message but it may 
also fail if the board design does not work with this workaround. I can 
add this if you think this is good to have.

>> You mentioned there is some existing logic to rewrite messages to match
>> driver constraints in the core driver.  I didn't see it when I did a quick
>> search on spi.c. I will take a deep look into the file. But if you can point
>> me where this logic is so I can be sure that I am looking at the right place
>> and will double check if this can be done or not in the core level.  Thanks!
> 
> spi_replace_transfers().
> 
Okay I saw this function is used by spi_split_transfers_maxsize which a 
few drivers use to limit the transfer size and it make sense.  I can 
come up something like spi_merge_transfers to be used by my driver's 
prepend function.  But it has the same issue I mentioned early as the 
these tx, rx transfers have the dependency on the order they present in 
the original transfer list for my prepend function to work.  And for the 
same reason, it won't be generally useful for other drivers.


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]