From: Eugeniu Rosca <erosca@de.adit-jv.com> To: Wolfram Sang <wsa@the-dreams.de> Cc: Eugeniu Rosca <erosca@de.adit-jv.com>, Ulf Hansson <ulf.hansson@linaro.org>, Adrian Hunter <adrian.hunter@intel.com>, Wolfram Sang <wsa+renesas@sang-engineering.com>, <linux-mmc@vger.kernel.org>, Linus Walleij <linus.walleij@linaro.org>, Mathieu Malaterre <malat@debian.org>, Pavel Machek <pavel@ucw.cz>, <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>, Eugeniu Rosca <roscaeugeniu@gmail.com> Subject: Re: [PATCH 1/3] dt-bindings: mmc: Add 'fixed-emmc-driver-type-hs{200,400}' Date: Tue, 5 Nov 2019 09:32:13 +0100 [thread overview] Message-ID: <20191105083213.GA24603@vmlxhi-102.adit-jv.com> (raw) In-Reply-To: <20191105062223.GB1048@kunai> Hi Wolfram, On Tue, Nov 05, 2019 at 07:22:23AM +0100, Wolfram Sang wrote: > Hi Eugeniu, > > thanks for this work! Thanks for the prompt response. Very much appreciated. > > > A certain eMMC manufacturer provided below requirement: > > ---snip--- > > Use "drive strength" value of 4 or 1 for HS400 or 0 for HS200. > > ---snip--- > > I see. > > > The existing "fixed-emmc-driver-type" property [1] is the closest one > > to implement the above, but it falls short due to being unable to define > > two values to differentiate between HS200 and HS400 (both modes may be > > supported by the same non-removable MMC device). > > > > To allow users to set a preferred HS200/HS400 "drive strength", provide > > two more bindings inspired from [1]: > > - fixed-emmc-driver-type-hs200 > > - fixed-emmc-driver-type-hs400 > > Main question before looking at the code: Can't we just extend the > existing binding with an optional second parameter? That's a great question/proposal, but before pushing the v2 right away, I would like to first share some thoughts. > minItems: 1 > maxItems: 2 > > I tend to favour this approach... The first question which pops up in my mind is related to the meaning of each item. The option which I envision based on your proposal is: * minItems: 1 * maxItems: 2 * Item[0]: Presumably equivalent to the current "fixed-emmc-driver-type", i.e. the strength value applied in both HS200 and HS400 modes. * Item[1] (optional): Presumably equivalent to "fixed-emmc-driver-type-hs400" proposed in this series. If this element is provided, the first one should likely change its role and become an equivalent of "fixed-emmc-driver-type-hs200" from this series. + Pro: Full backward compatibility. No need to touch the existing users of "fixed-emmc-driver-type". - Con: Not sure we have such DT bindings which dynamically change their semantics based on the usage pattern. - Con: Can't easily achieve the same flexibility as accomplished in this series. For example, current implementation allows users to define each of the three parameters (i.e. HSx00-agnostic drive strength, HS200 and HS400 specific drive strengths) individually, as well as in all possible combinations. This might be needed if, in certain HSx00 mode, users still need to rely on the RAW/unmodified drive strength. I am unsure if/how this can be achieved with an array OF property with a constant or variable number of elements (I try to sketch one solution below). One option to achieve a similar degree of flexibility by using an array OF property (instead of several u32 properties) would be to agree on a convention based on magic values, i.e. below DT one-liner could be an example of providing solely the "fixed-emmc-driver-type-hs200" value (based on the agreement that 0xFF values are discarded by the driver): fixed-emmc-driver-type = <0xFF 0x1 0xFF>; > > > For more details about eMMC I/O driver strength types, see Jedec spec. > > Keep "fixed-emmc-driver-type" in place for backward compatibility. > > If we decide for the path proposed here, should the old binding be > deprecated then? I can either zap "fixed-emmc-driver-type" or extend its type and meaning, depending on the feedback from the reviewers. Looking forward to any comments and suggestions. > > Happy hacking, > > Wolfram Thanks. -- Best Regards, Eugeniu
WARNING: multiple messages have this Message-ID (diff)
From: Eugeniu Rosca <erosca@de.adit-jv.com> To: Wolfram Sang <wsa@the-dreams.de> Cc: Eugeniu Rosca <erosca@de.adit-jv.com>, Ulf Hansson <ulf.hansson@linaro.org>, Adrian Hunter <adrian.hunter@intel.com>, Wolfram Sang <wsa+renesas@sang-engineering.com>, linux-mmc@vger.kernel.org, Linus Walleij <linus.walleij@linaro.org>, Mathieu Malaterre <malat@debian.org>, Pavel Machek <pavel@ucw.cz>, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Eugeniu Rosca <roscaeugeniu@gmail.com> Subject: Re: [PATCH 1/3] dt-bindings: mmc: Add 'fixed-emmc-driver-type-hs{200,400}' Date: Tue, 5 Nov 2019 09:32:13 +0100 [thread overview] Message-ID: <20191105083213.GA24603@vmlxhi-102.adit-jv.com> (raw) In-Reply-To: <20191105062223.GB1048@kunai> Hi Wolfram, On Tue, Nov 05, 2019 at 07:22:23AM +0100, Wolfram Sang wrote: > Hi Eugeniu, > > thanks for this work! Thanks for the prompt response. Very much appreciated. > > > A certain eMMC manufacturer provided below requirement: > > ---snip--- > > Use "drive strength" value of 4 or 1 for HS400 or 0 for HS200. > > ---snip--- > > I see. > > > The existing "fixed-emmc-driver-type" property [1] is the closest one > > to implement the above, but it falls short due to being unable to define > > two values to differentiate between HS200 and HS400 (both modes may be > > supported by the same non-removable MMC device). > > > > To allow users to set a preferred HS200/HS400 "drive strength", provide > > two more bindings inspired from [1]: > > - fixed-emmc-driver-type-hs200 > > - fixed-emmc-driver-type-hs400 > > Main question before looking at the code: Can't we just extend the > existing binding with an optional second parameter? That's a great question/proposal, but before pushing the v2 right away, I would like to first share some thoughts. > minItems: 1 > maxItems: 2 > > I tend to favour this approach... The first question which pops up in my mind is related to the meaning of each item. The option which I envision based on your proposal is: * minItems: 1 * maxItems: 2 * Item[0]: Presumably equivalent to the current "fixed-emmc-driver-type", i.e. the strength value applied in both HS200 and HS400 modes. * Item[1] (optional): Presumably equivalent to "fixed-emmc-driver-type-hs400" proposed in this series. If this element is provided, the first one should likely change its role and become an equivalent of "fixed-emmc-driver-type-hs200" from this series. + Pro: Full backward compatibility. No need to touch the existing users of "fixed-emmc-driver-type". - Con: Not sure we have such DT bindings which dynamically change their semantics based on the usage pattern. - Con: Can't easily achieve the same flexibility as accomplished in this series. For example, current implementation allows users to define each of the three parameters (i.e. HSx00-agnostic drive strength, HS200 and HS400 specific drive strengths) individually, as well as in all possible combinations. This might be needed if, in certain HSx00 mode, users still need to rely on the RAW/unmodified drive strength. I am unsure if/how this can be achieved with an array OF property with a constant or variable number of elements (I try to sketch one solution below). One option to achieve a similar degree of flexibility by using an array OF property (instead of several u32 properties) would be to agree on a convention based on magic values, i.e. below DT one-liner could be an example of providing solely the "fixed-emmc-driver-type-hs200" value (based on the agreement that 0xFF values are discarded by the driver): fixed-emmc-driver-type = <0xFF 0x1 0xFF>; > > > For more details about eMMC I/O driver strength types, see Jedec spec. > > Keep "fixed-emmc-driver-type" in place for backward compatibility. > > If we decide for the path proposed here, should the old binding be > deprecated then? I can either zap "fixed-emmc-driver-type" or extend its type and meaning, depending on the feedback from the reviewers. Looking forward to any comments and suggestions. > > Happy hacking, > > Wolfram Thanks. -- Best Regards, Eugeniu
next prev parent reply other threads:[~2019-11-05 8:32 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-11-05 5:50 [PATCH 1/3] dt-bindings: mmc: Add 'fixed-emmc-driver-type-hs{200,400}' Eugeniu Rosca 2019-11-05 5:50 ` Eugeniu Rosca 2019-11-05 5:50 ` [PATCH 2/3] mmc: host: Compress 'fixed-emmc-driver-type' handling Eugeniu Rosca 2019-11-05 5:50 ` Eugeniu Rosca 2019-11-05 5:50 ` [PATCH 3/3] mmc: core: Add 'fixed-emmc-driver-type-hs{200,400}' Eugeniu Rosca 2019-11-05 5:50 ` Eugeniu Rosca 2019-11-05 6:22 ` [PATCH 1/3] dt-bindings: mmc: " Wolfram Sang 2019-11-05 8:32 ` Eugeniu Rosca [this message] 2019-11-05 8:32 ` Eugeniu Rosca 2019-11-07 0:39 ` Rob Herring 2019-11-12 21:19 ` Wolfram Sang 2019-11-12 23:11 ` Linus Walleij 2019-11-12 23:11 ` Linus Walleij 2019-11-14 10:46 ` Ulf Hansson 2019-11-06 11:07 ` Linus Walleij 2019-11-06 11:07 ` Linus Walleij 2019-11-11 22:25 ` Eugeniu Rosca 2019-11-11 22:25 ` Eugeniu Rosca 2019-11-12 23:08 ` Linus Walleij 2019-11-12 23:08 ` Linus Walleij
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20191105083213.GA24603@vmlxhi-102.adit-jv.com \ --to=erosca@de.adit-jv.com \ --cc=adrian.hunter@intel.com \ --cc=devicetree@vger.kernel.org \ --cc=linus.walleij@linaro.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mmc@vger.kernel.org \ --cc=malat@debian.org \ --cc=pavel@ucw.cz \ --cc=roscaeugeniu@gmail.com \ --cc=ulf.hansson@linaro.org \ --cc=wsa+renesas@sang-engineering.com \ --cc=wsa@the-dreams.de \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.