lkml.org 
[lkml]   [2022]   [Jul]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH 2/5] dt-bindings: mmc: sdhci-msm: constrain reg-names at least for one variant
    Hi,

    On Thu, Jul 7, 2022 at 1:04 AM Krzysztof Kozlowski
    <krzysztof.kozlowski@linaro.org> wrote:
    >
    > The entries in arrays must have fixed order, so the bindings and Linux
    > driver expecting various combinations of 'reg' addresses was never
    > actually conforming to guidelines.
    >
    > Specifically Linux driver always expects 'core' reg entry as second
    > item, but some DTSes are having there 'cqhci'.

    This is a bit misleading and makes it sound like we've got a bug. In
    truth the Linux driver looks at the compatible string. If it sees any
    compatible listed as "v5" (or a slight variant of v5 to handle a
    workaround for sc7180 / sdm845) then it _doesn't_ expect 'core' reg as
    the second entry. See the variable `mci_removed`. The old bindings
    ".txt" file also had this to say:

    For SDCC version 5.0.0, MCI registers are removed from SDCC
    interface and some registers are moved to HC. New compatible
    string is added to support this change - "qcom,sdhci-msm-v5".

    So I guess that means this is the documentation for all of the
    combinations you have listed:

    * hc only - v5 controller w/out CQE / ICE

    * hc + core - v4 controller w/out CQE / ICE

    * hc + cqhci - v5 controller w/ CQE and w/out ICE

    * hc + cqhci + ice - v5 controller w/ CQE / ICE

    * hc + core + cqhci + ice - v4 controller w/ CQE / ICE

    Said another way, before v5 the "core" range existed. After v5 it
    apparently doesn't so there's no way we could have specified it.

    You'll notice that one of the options above implies that a v4
    controller (with "core" specified) can have CQE and ICE. Is this
    actually true, or was it a misunderstanding in the .txt to .yaml
    conversion?

    If it's true that a v4 controller can have CQE and ICE then your patch
    is wrong in asserting that v4 controllers have only "hc" and "core".

    If a v4 controller _can't_ have CQE and ICE then your patch is right
    but incomplete. It should also be removing the option:
    - const: hc
    - const: core
    - const: cqhci
    - const: ice

    I am not intimately familiar with Qualcomm MMC controller history.
    That being said, the old .txt file said:

    - CQE register map (Optional, CQE support is present on SDHC
    instance meant
    for eMMC and version v4.2 and above)

    To me this implies that a v4 controller could _at least_ have "cqhci".
    I dunno about "ice". I seem to recall that this was the argument for
    why the driver had to use reg-names to begin with and why the driver
    looks for "cqhci" by name.

    -Doug

    \
     
     \ /
      Last update: 2022-07-07 16:33    [W:8.302 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site