lkml.org 
[lkml]   [2023]   [May]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH v2 1/2] dt-bindings: phy: add mediatek mipi csi driver v 0.5
    Date
    Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> writes:

    > On 16/05/2023 23:31, Kevin Hilman wrote:
    >
    >>> Third is to use versioned IP blocks.
    >>>
    >>> The second case also would work, if it is applicable to you (you really
    >>> have fallback matching all devices). Third solution depends on your
    >>> versioning and Rob expressed dislike about it many times.
    >>>
    >>> We had many discussions on mailing lists, thus simplifying the review -
    >>> I recommend the first choice. For a better recommendation you should say
    >>> a bit more about the block in different SoCs.
    >>
    >> I'll try to say a bit more about the PHY block, but in fact, it's not
    >> just about differences between SoCs. On the same SoC, 2 different PHYs
    >> may have different features/capabilities.
    >>
    >> For example, on MT8365, There are 2 PHYs: CSI0 and CSI1. CSI0 can
    >> function as a C-PHY or a D-PHY, but CSI1 can only function as D-PHY
    >> (used as the example in the binding patch[1].) On another related SoC,
    >> there are 3 PHYs, where CSI0 is C-D but CSI1 & CSI2 are only D.
    >>
    >> So that's why it seems (at least to me) that while we need SoC
    >> compatible, it's not enough. We also need properties to describe
    >> PHY-specific features (e.g. C-D PHY)
    >
    > I recall the same or very similar case... It bugs me now, but
    > unfortunately I cannot find it.
    >
    >>
    >> Of course, we could rely only on SoC-specific compatibles describe this.
    >> But then driver will need an SoC-specific table with the number of PHYs
    >> and per-PHY features for each SoC encoded in the driver. Since the
    >> driver otherwise doesn't (and shouldn't, IMHO) need to know how many
    >> PHYs are on each SoC, I suggested to Julien that perhaps the additional
    >> propery was the better solution.
    >
    > Phys were modeled as separate device instances, so you would need
    > difference in compatible to figure out which phy is it.
    >
    > Other way could be to create device for all phys and use phy-cells=1.
    > Whether it makes sense, depends on the actual datasheet - maybe the
    > split phy per device is artificial? There is one PHY block with two
    > address ranges for each PHY - CSI0 and CSI1 - but it is actually one
    > block? You should carefully check this because once design is chosen,
    > you won't be able to go back to other and it might be a problem (e.g.
    > there is some top-level block for powering on all CSI instances).

    We're pretty sure these are multiple instances of the IP block as they
    can operate completely independently.

    >>
    >> To me it seems redundant to have the driver encode PHYs-per-SoC info,
    >> when the per-SoC DT is going to have the same info, so my suggestion was
    >> to simplify the driver and have this kind of hardware description in the
    >> DT, and keep the driver simple, but we are definitely open to learning
    >> the "right way" of doing this.
    >
    > The property then is reasonable. It should not be bool, though, because
    > it does not scale. There can be next block which supports only D-PHY on
    > CSI0 and C-PHY on CSI1? Maybe some enum or list, depending on possible
    > configurations.

    OK, looks like include/dt-bindings/phy/phy.y already has

    #define PHY_TYPE_DPHY 10
    #define PHY_TYPE_CPHY 11

    we'll add a PHY_TYPE_CDPHY and use that. Sound reasonable?

    Kevin

    \
     
     \ /
      Last update: 2023-05-22 21:17    [W:4.183 / U:2.328 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site