[v5 3/5] dt-bindings: mfd: Add aspeed pwm-tach binding

Billy Tsai billy_tsai at aspeedtech.com
Wed Jun 7 16:26:19 AEST 2023


        On 06/06/2023 16:06, Patrick Williams wrote:
        >> On Tue, Jun 06, 2023 at 12:49:04PM +0200, Krzysztof Kozlowski wrote:
        >>
        >> Hi Krzysztof,
        >>
        >> Thank you for reviewing this from Billy.
        >>
        >> The Aspeed chip is heavily used by the OpenBMC community and the 2600
        >> has been used in production systems for almost 2 years now.  Many
        >> companies are having to carry previous versions of these as patches, and
        >> some of the APIs changed since the last revision from Billy.  So, I had
        >> asked him to submit the latest patch set with as many revisions as he
        >> understood what to change, since the conversation seemed to have died
        >> since last time he submitted.
        >>
        >> I don't believe Billy is intentionally ignoring your feedback and he is
        >> motivated to get this patch set wrapped up into an acceptable state.
        >>
        >>> On 06/06/2023 11:45, Billy Tsai wrote:
        >>
        >>> NAK. You got here clear comment. You cannot have simple MFD with
        >>> resources. It is not simple anymore.
        >>>
        >>
        >> In fairness, Billy asked for clarification from you on this point and didn't
        >> receive it.
        >>
        >> https://lore.kernel.org/lkml/24DD1FEB-95F3-47BE-BE61-8B0E6FBDE20F@aspeedtech.com/

        > I gave the instruction what Billy should do:

        > https://lore.kernel.org/lkml/41500a04-b004-0e2c-20a1-3a3092b90e6d@linaro.org/

        > What about other ignored comments? About subject, quotes and more? Even
        > if this one was unclear, then why ignoring all the rest?

It's possible that there was some confusion regarding your message. I apologize for any misunderstanding.
About the subject: I apologize for the misunderstanding. I just drop the redundant "bindings" in the commit message.
About the quotes: I believe the issue was simply related to the order of the patches, and I have resolved it. Did I misunderstand?
About the Missing description:

> +patternProperties:
> +  "^fan@[a-z0-9]+$":
> +    type: object

> Missing description. But more important - why do you have such child
> nodes? Your example does not have them. What's the point? Do you expect
> different number of fans per one device (one compatible)?

In this patch series, I have included examples and descriptions to provide additional information.
The child node is used to enable the channel of this tach controller.
I expect that the dts will include information regarding the number of fans connected to the board and their corresponding channels.

        >>
        >> He felt what he was trying to accomplish met the documented
        >> expectations.  Are there some changes that need to be done in mfd.txt to
        >> further clarify when to use it and when not to?

        > I think mfd.txt clearly states:
        > "For more complex devices, when the nexus driver has to
        > probe registers to figure out what child devices exist etc, this should
        > not be used. In the latter case the child devices will be determined by
        > the operating system."

About the mfd:
For our pwm and tach devices, there is no need to check/apply any hardware register from parent to determine child’s existence or functional.
They don’t have any dependency on the parent node. In fact, it doesn’t require a specific driver to bind with the "aspeed,ast2600-pwm-tach" label. Their purpose is solely to share the same clock, reset phandle and base address. The main reason for using simple-mfd in this case is because these two independent devices share the same base address. In fact, I can relocate the clock and reset configurations to the child nodes rather than the parent node.  In this case, I still can't use simple-mfd?

I appreciate your assistance and review.

Best Regards,
Billy Tsai
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/linux-aspeed/attachments/20230607/30c6b1f7/attachment.htm>


More information about the Linux-aspeed mailing list