lkml.org 
[lkml]   [2022]   [Jun]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH net-next 01/28] dt-bindings: phy: Add QorIQ SerDes binding
    From
    On 20/06/2022 20:51, Sean Anderson wrote:
    > On 6/20/22 2:21 PM, Krzysztof Kozlowski wrote:
    >>>>> - samsung_usb2_phy_config in drivers/phy/samsung/
    >>>>
    >>>> This one is a good example - where do you see there compatibles with
    >>>> arbitrary numbers attached?
    >>>
    >>> samsung_usb2_phy_of_match in drivers/phy/samsung/phy-samsung-usb2.c
    >>>
    >>> There is a different compatible for each SoC variant. Each compatible selects a struct
    >>> containing
    >>>
    >>> - A list of phys, each with custom power on and off functions
    >>> - A function which converts a rate to an arbitrary value to program into a register
    >>>
    >>> This is further documented in Documentation/driver-api/phy/samsung-usb2.rst
    >>
    >> Exactly, please follow this approach. Compatible is per different
    >> device, e.g. different SoC variant. Of course you could have different
    >> devices on same SoC, but "1" and "2" are not different devices.
    >
    > (in this case they are)

    In a meaning of descriptive compatible - it's not.

    >>>
    >>> - For some SerDes on the same SoC, these fields are reserved
    >>
    >> That all sounds like quite different devices, which indeed usually is
    >> described with different compatibles. Still "xxx-1" and "xxx-2" are not
    >> valid compatibles. You need to come with some more reasonable name
    >> describing them. Maybe the block has revision or different model/vendor.
    >
    > There is none AFAIK. Maybe someone from NXP can comment (since there are many
    > undocumented registers).

    Maybe it's also possible to invent some reasonable name based on
    protocols supported? If nothing comes then please add a one-liner
    comment explaining logic behind 1/2 suffix.

    >>> The compatibles suggested were "fsl,ls1046-serdes-1" and -2. As noted above, these are separate
    >>> devices which, while having many similarities, have different register layouts and protocol
    >>> support. They are *not* 100% compatible with each other. Would you require that clock drivers
    >>> for different SoCs use the same compatibles just because they had the same registers, even though
    >>> the clocks themselves had different functions and hierarchy?
    >>
    >> You miss the point. Clock controllers on same SoC have different names
    >> used in compatibles. We do not describe them as "vendor,aa-clk-1" and
    >> "vendor,aa-clk-2".
    >>
    >> Come with proper naming and entire discussion might be not valid
    >> (although with not perfect naming Rob might come with questions). I
    >> cannot propose the name because I don't know these hardware blocks and I
    >> do not have access to datasheet.
    >>
    >> Other way, if any reasonable naming is not possible, could be also to
    >> describe the meaning of "-1" suffix, e.g. that it does not mean some
    >> index but a variant from specification.
    >
    > The documentation refers to these devices as "SerDes1", "SerDes2", etc.
    >
    > Wold you prefer something like
    >
    > serdes0: phy@1ea0000 {
    > compatible = "fsl,ls1046a-serdes";
    > variant = <0>;
    > };

    No, it's the same problem, just embeds compatible in different property.

    Best regards,
    Krzysztof

    \
     
     \ /
      Last update: 2022-06-21 09:14    [W:2.449 / U:0.652 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site