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

[Mark Brown <broonie@kernel.org>]

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

On Fri, Nov 18, 2022 at 11:16:22PM +0900, Kunihiko Hayashi wrote:
> 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.

Yes, sounds good.

> > > +    $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.

Ah, so this is some additional thing on top of dummy cycles?  I'd
not realised that.  It probably wants to be added into spi-mem I
guess.

> > > +  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.

That also works, someone can always add additional support later
when they have a concrete use case.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]