AW: AW: [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes PHY binding

[markus.stockhausen at gmx.de]

> -----Ursprüngliche Nachricht-----
> Von: Krzysztof Kozlowski <krzk at kernel.org> 
> Gesendet: Dienstag, 8. Oktober 2024 08:17
> An: markus.stockhausen at gmx.de; linux-phy at lists.infradead.org; chris.packham at alliedtelesis.co.nz; devicetree at vger.kernel.org
> Betreff: Re: AW: [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes PHY binding
>
> On 08/10/2024 07:38, markus.stockhausen at gmx.de wrote:
> >> -----Ursprüngliche Nachricht-----
> >> Von: Krzysztof Kozlowski <krzk at kernel.org>
> >> Gesendet: Montag, 7. Oktober 2024 21:26
> >> An: Markus Stockhausen <markus.stockhausen at gmx.de>; 
> >> linux-phy at lists.infradead.org; chris.packham at alliedtelesis.co.nz; 
> >> devicetree at vger.kernel.org
> >> Betreff: Re: [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes 
> >> PHY binding
> >>
> >> ... and still not tested. Sending untested code is waste of our time.
> > 
> > Hi Krzysztof,
> > 
> > appreciate your feedback and I do not want to waste your time. My 
> > fixes where a mix of your feedback and some half-baked "make 
> > dt_binding_check" feedbacks (because packages where missing). My fault and sorry fort he noise.
> > 
> > To get next version in better shape two questions regarding your feedback:
> > 
> > 1. "Messed wrapping": According to checkpatch 100 chars/line are accepted. 
> > So I designed the comments in the driver. Does devicetree differ from that?
>
> checkpatch is not a coding style. I asked to follow coding style, please read entire document in Documentation/process.

Understood.

> > 
> > 2 "Bindings vs drivers". The idea about controlled ports came from other bindings.
>
> Entire property description speaks about driver, not bindings.
>
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tre
> > e/Documentation/devicetree/bindings/interrupt-controller/st,stih407-ir
> > q-syscfg.yaml?h=v6.12-rc2
>
> stih is rather poor example to use. The property was added in 2015 (!) without review (!!!).
>
>
> > E.g. st,invert-ext. Something like this will be needed in the future 
> > because the SerDes allow to swap polarity which must be changed 
> > depending on the switch design. How to do this?
>
> I do not understand the hardware aspect discussed in the property description... probably because there is no hardware description at all, but instead you speak about driver.
>
> I do not understand how polarity has anything to do with U-Boot configuring serdes.

Maybe my lack of knowledge in platform driver programming or the naming
conventions leads to confusion. I'm searching for knobs to control the behaviour 
of the SerDes depending on the hardware. Two examples are (more may come):

- "ignore SerDes X": because the provided patch sequence confuses the SerDes
and overwrites registers with wrong values that vendor patched U-Boot has setup
correctly before. 

- "reverse polarity of SerDes X": same goes here. Some boards need inverted
signalling on some of the SerDes to work properly. This must be configurable
somehow.

Looking at some more modern implementation/documentation I need soemthing 
like in realtek,usb2phy.yaml - e.g. realtek,driving-level-compensate.

Should I just leave "driver" out of the description?

Best regards.

Markus