Re: [PATCH v3 1/2] dt-bindings: media: i2c: Add onsemi AR0234 image sensor binding

[Aleksandr Chubuchnyi <chubuchnyi@gmail.com>]

Hi,

Three things that may help: the concrete colour/mono difference, a close
precedent in-tree, and a piece of earlier review that has not come up.

> What are the actual programming differences between sc, sm and "none"
> variants?

There are two hardware models; the third string is not one of them.
From patch 2/2: the models have distinct chip ids, 0x0a56 and 0x1a56,
read from CHIP_VERSION (0x3000), and an unrecognised id fails probe.
The only register write conditioned on the model is DIGITAL_TEST bit 7,
MONO_CHROME_OPERATION. The model also selects the advertised bus codes,
Y8/Y10 versus SGRBG8/SGRBG10, and the mode table exposes the 10-to-8-bit
DPCM code for colour only - though that last one is arguably a
media-bus-format gap, as there is no mono DPCM code, rather than a
sensor difference.

The suffix-less "onnn,ar0234cs" entry carries no match data, so the
model read from the chip is kept. The cssc/cssm entries do carry match
data, and on a mismatch the driver warns and uses the DT-selected one.

> So all devices are compatible? Then why compatibility is not expressed?

sony,imx678.yaml went through this exact question in May. It was posted
as a flat enum of three strings, with a description noting the variants
could be detected at runtime:

  compatible:
    enum:
      - sony,imx678
      - sony,imx678-aamr
      - sony,imx678-aaqr

Conor's objection was that a DT naming both the specific and the generic
string would then fail validation:

  https://lore.kernel.org/all/20260520-crusher-species-cf707a9a8b46@spud/

and v4 changed it to the form now in tree, with the changelog entry
"Follow Conor's suggestion of mandating both the specific and generic
device name in the compatible property":

  compatible:
    items:
      - enum:
          - sony,imx678-aamr
          - sony,imx678-aaqr
      - const: sony,imx678

Note that is the mandatory form rather than the oneOf v2 used - there
the generic string alone is not valid.

The other colour/mono bindings in that directory use a flat enum:
sony,imx296, st,vd55g1 and st,vd56g3 all describe the split in the
binding text, and aptina,mt9p031 does not. sony,imx290 uses a oneOf,
but of five model strings plus a deprecated standalone generic.

The v2 fallback form did go through dt-bindings review. Rob quoted it
down to the enum line:

  compatible:
    oneOf:
      - const: onnn,ar0234cs
      - items:
          - enum: ["onnn,ar0234cssc", "onnn,ar0234cssm"]

replied inline with

  - enum:
      - onnn,ar0234cssc
      - onnn,ar0234cssm

  Please explain somewhere what the differences between the 3 cases are.

and the quotation then resumes with "- const: onnn,ar0234cs":

  https://lore.kernel.org/all/20260812210309.GA972604-robh@kernel.org/

So that replaced the flow-style enum with a block one and left the
oneOf/items structure standing. Reverting to the v2 shape would not be
going against that review.

That leaves the question the thread has not settled: whether a bare
generic compatible is meant to be valid. Laurent listed the use cases
the model-specific strings serve, on the RFC -

  https://lore.kernel.org/linux-media/20260505163713.GE1547435@killaraus.ideasonboard.com/

avoiding a sensor power-up at boot when the module is known at build
time, and a runtime sanity check. Of those only the sanity check is
delivered today, since probe powers the sensor up and reads the id
before it looks at the match data, so a model-specific compatible does
not currently save anything at boot. He proposed both forms in that
message, and concluded "I don't think we've decided on a recommended
practice."

If one DT is meant to serve interchangeable colour and mono modules,
the v2 oneOf expresses that. If conforming DTs are expected to name the
model, the mandatory imx678 form fits, and the driver can still tolerate
generic-only DTs. Either way it seems worth saying which one in the
binding.

Thanks,
Aleksandr Chubuchnyi