Re: [PATCH 1/2] dt-bindings: spi: Add Socionext F_OSPI controller bindings

[Kunihiko Hayashi <hayashi.kunihiko@socionext.com>]

Hi Mark,

Thank you for your comment.

On 2022/11/18 20:39, Mark Brown wrote:
> On Fri, Nov 18, 2022 at 09:59:03AM +0900, Kunihiko Hayashi wrote:
> 
>> +  socionext,cs-start-cycle:
>> +  socionext,cs-end-cycle:
>> +  socionext,cs-deassert-clk-cycle:
> 
> These are all generic SPI properties so we should add them
> generically, on the device rather than the controller since this
> is something that might vary per client device.  There was also a
> core function spi_set_cs_timing() which was in earlier versions
> and is about to get reintroduced.

So I understand you mean that these properties should be defined like
spi-peripheral-props.yaml for the devices.

If yes, I'll drop these properties once and define our vendor-specific
"peripheral-props" in the next series.

>> +  socionext,alternative-byte:
>> +    description:
>> +      Specify the extra bytes to transfer after address transfer.
> 
> This just doesn't seem like something the controller should be
> worrying about, the device should just send whatever the device
> needs sending.

This seems to be a feature added for the special devices,
so this should be device matter.

>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>> +    items:
>> +      - description: the number of bytes to transfer
>> +        maximum: 4
>> +      - description: value to transfer
>> +        default: 0
>> +      - description: bit-width to transfer
>> +        enum: [0, 1, 2, 4, 8]
> 
> This is also something SPI device should set up, as far as I can
> tell this should be set vis spi_mem_op.dummy.nbytes.

Yes, but the controller also supports dummy cycles, and can send
extra bytes before the dummy cycles.

>> +  socionext,data-swap-2byte:
>> +    description:
>> +      Indicates swap byte order per 2-bytes.
>> +    type: boolean
> 
>> +  socionext,data-swap-4byte:
>> +    description:
>> +      Indicates swap byte order per 4-bytes.
>> +    type: boolean
> 
> Again these should be set by the device.  I think these should be
> set based on a combination of bits per word and if the host is in
> big endian or little endian mode.

I see. This feature is complicated to use, so I'll not add it here.

Thank you,

---
Best Regards
Kunihiko Hayashi