| Message ID | 20191105055015.23656-1-erosca@de.adit-jv.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [1/3] dt-bindings: mmc: Add 'fixed-emmc-driver-type-hs{200,400}' | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | "total: 0 errors, 1 warnings, 32 lines checked" |
| robh/dt-meta-schema | success |
Hi Eugeniu, thanks for this work! > 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? minItems: 1 maxItems: 2 I tend to favour this approach... > 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? Happy hacking, Wolfram
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.
Hi Eugeniu, thanks for your patch! On Tue, Nov 5, 2019 at 6:50 AM Eugeniu Rosca <erosca@de.adit-jv.com> wrote: > A certain eMMC manufacturer provided below requirement: > ---snip--- > Use "drive strength" value of 4 or 1 for HS400 or 0 for HS200. > ---snip--- > > 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 I am sorry that I do not quite understand but as pin control maintainer I am of course triggered by the talk about selecting "drive strength". In my book this means that the pad driver on the chip, driving the line low/high with push-pull (totempole output, usually) is connecting more driver stages, usually just shunting in more totempoles. (Ref https://en.wikipedia.org/wiki/Push%E2%80%93pull_output) If say one totempole gives 2mA drive strength then 4 totempoles gives 8mA drive strength. Are we on the same page here that this is what physically happens? Usually selection of drive strength is done with the pin control framework, so this would need to be backed by code (not in this patch set) that select pin control states that reconfigure the SoC pad drivers to use the requested strength. Alternatively, the (e)MMC block would implement this control directly, but I doubt it. Please clarify which hardware is eventually going to provide the drive strength alteration, because I just don't see it in the patch set. Is the assumption that the (e)MMC hardware will do this autonomously or something? That may be a pecularity to the hardware you're using in that case. I find the fixed-emmc-driver-type-* assignment a but puzzling to be honest, isnt' the driver device tree already specifying what the hardware can do with all of these: mmc-ddr-1_2v mmc-ddr-1_8v mmc-ddr-3_3v mmc-hs200-1_2v mmc-hs200-1_8v mmc-hs400-1_2v mmc-hs400-1_8v mmc-hs400-enhanced-strobe If the host is already specifying mmc-hs200-* or mmc-hs400-* then certainly it should be implied that the host supports hs200 and hs400 and there is no need for the fixed-emmc-driver-type-hs* properties. The code detects when to use each mode and that is when you can insert the code to switch drive strengths, whether using the pin control framework or something else. So to me it seems these DT properties are just introduced to hammer down a certain usecase instead of letting the code with the help of DT speed capabilities flags determine what speed is to be used and select the appropriate drive strength. Yours, Linus Walleij
On Tue, Nov 05, 2019 at 09:32:13AM +0100, Eugeniu Rosca wrote: > 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? I was thinking the same thing... > > 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>; I don't understand why you have 3 values instead of 2. I would just use -1 if you want to ignore an entry. If that's the common case, then I'd stick with what you originally proposed. If rare, then I think an array is the better route. Rob
Hi Linus, On Wed, Nov 06, 2019 at 12:07:38PM +0100, Linus Walleij wrote: > Hi Eugeniu, > > thanks for your patch! Thanks for your comments. > > On Tue, Nov 5, 2019 at 6:50 AM Eugeniu Rosca <erosca@de.adit-jv.com> wrote: > > > A certain eMMC manufacturer provided below requirement: > > ---snip--- > > Use "drive strength" value of 4 or 1 for HS400 or 0 for HS200. > > ---snip--- > > > > 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 > > I am sorry that I do not quite understand but as pin control maintainer I > am of course triggered by the talk about selecting "drive strength". > > In my book this means that the pad driver on the chip, driving the > line low/high with push-pull (totempole output, usually) is connecting > more driver stages, usually just shunting in more totempoles. > (Ref https://en.wikipedia.org/wiki/Push%E2%80%93pull_output) > > If say one totempole gives 2mA drive strength then 4 totempoles > gives 8mA drive strength. > > Are we on the same page here that this is what physically happens? This matches my view with below amendments: - Your passage seems to describe a single-duplex communication (one end is always a sender and the other one is always a receiver). Since the CMD and DAT[0-7] eMMC lines are bidirectional (carrying half-duplex data transfers), this is what seems to justify the "drive(r) strength/type" feature/setting to be present on both host controller and eMMC device ends (which does happen in real life). - I am unsure whether to endorse the "totempole output" as being the usual foundation for how "drive strength" is really implemented in the modern CMOS ICs, based on the following: - All eMMC Jedec specs mention "push-pull" for CMD/DAT[0-7] - All eMMC device datasheets I could find reference "push pull" and none mentions "totem pole" for CMD/DAT[0-7] - The "totem pole" topology seems to originate from and be much more popular in the TTL/BJT world, where it tries to harness the symmetry of two NPN transistors, replacing the PNP-NPN pair used in the bipolar "push-pull" configuration [1-2]. - Jedec calls totem-pole "a bipolar output" (i.e. TTL/BJT) [3] - Jedec claims [3] that "vanilla" tottempole doesn't support tristate/hi-Z outputs, making it impossible to connect several such circuits in parallel, while we assume the latter to be a hard prerequisite for sourcing programmable amounts of current. - Some users say that "CMOS outputs are generally push-pull" [4] - TI states [5] that the "MOSFET equivalent of the bipolar totempole driver [..] is rarely implemented" Abstracting from the above, I agree that a programmable "drive strength" is likely achieved by connecting several tristate-capable output circuits in a "wired OR", as exemplified for Raspberry Pi in [6]. > > Usually selection of drive strength is done with the pin control > framework, so this would need to be backed by code (not in this > patch set) that select pin control states that reconfigure the > SoC pad drivers to use the requested strength. That's true. In the same context of overcoming HS400 issues, our SoC vendor suggested adjusting the "drive-strength" binding, added in: - 7db9af4b6e41be ("pinctrl: add function to parse generic pinconfig properties from a dt node") - 3caa7d8c3f03ad ("pinctrl: sh-pfc: Add drive strength support") > Alternatively, the (e)MMC block would implement this control > directly, but I doubt it. There _is_ a "drive strength" specific to eMMC device and the justification for it to exist has been made above. According to JESD84-B50.1 spec, the host controller is able to find the "drive strength" values supported by a particular eMMC device by reading the DRIVER_STRENGTH field of the Extended CSD. The host then may (if needed), make use of this value to set the "Driver Strength" parameter in the HS_TIMING field of the Extended CSD register. Essentially, current series gives the host controller a chance to limit the drive strength value written to HS_TIMING, if eMMC vendor decides that some of the values advertised in DRIVER_STRENGTH are forbidden or should be avoided in a specific bus speed mode (HS200/HS400). > Please clarify which hardware is eventually going to provide the > drive strength alteration, because I just don't see it in the patch > set. Is the assumption that the (e)MMC hardware will do this > autonomously or something? That may be a pecularity to the hardware > you're using in that case. Hopefully clarified above. > I find the fixed-emmc-driver-type-* assignment a but puzzling > to be honest, isnt' the driver device tree already specifying > what the hardware can do with all of these: > > mmc-ddr-1_2v > mmc-ddr-1_8v > mmc-ddr-3_3v JFTR, for these (<HS200) bus speed modes, the eMMC Jedec spec doesn't talk about changing eMMC's driver strength. That's probably because there are no signal integrity issues to be fixed in these low-speed modes. > mmc-hs200-1_2v > mmc-hs200-1_8v > mmc-hs400-1_2v > mmc-hs400-1_8v > mmc-hs400-enhanced-strobe Below is a quote from JESD84-B50.1 (10.5.4.1 Driver Types Definition): -> NOTE: Drive strength definitions are same for 1.8 V signaling level -> and for 1.2 V signaling level. I read the above as: -> "the driver strength is independent/agnostic on signaling level" > If the host is already specifying mmc-hs200-* or > mmc-hs400-* then certainly it should be implied that the > host supports hs200 and hs400 and there is no need for > the fixed-emmc-driver-type-hs* properties. Not sure I see the linkage between the cause and effect here. The host cannot infer anything about the supported drive strength values on eMMC side purely based on the mmc-hs200-*/mmc-hs400-* DT properties. > > The code detects when to use each mode and that is when > you can insert the code to switch drive strengths, whether using > the pin control framework or something else. As explained above, this series allows to customize the eMMC-specific drive strength. The eMMC vendor did not ask to make the SoC-side drive strength dependent on bus speed mode and that was not needed in the testing performed by the customer. > > So to me it seems these DT properties are just introduced to > hammer down a certain usecase instead of letting the code with the > help of DT speed capabilities flags determine what speed is to be used > and select the appropriate drive strength. Does this mean that the "fixed-emmc-driver-type" binding which pre-exists my series falls under the same sentence? Or is this only when customizing Wolfram's binding to HS200/HS400 bus speed mode? Note that there is no other objective in this series but to allow Linux to run on hardware which doesn't strictly follow its specification [7]. If you have any alternative ideas of how to follow the eMMC vendor's recommendation quoted in the description of this patch, I will happily review those. > > Yours, > Linus Walleij [1] https://electronics.stackexchange.com/q/358151/235971 [2] https://electronics.stackexchange.com/a/17819/235971 ---snip---- Due to the difference in N and P carrier mobilities, NPN and PNP transistors are never truly symmetric, and there were advantages to using NPN. [..] In CMOS logic, the N and P channel drivers are symmetric and the driver designs are truly complementary. ---snip---- [3] https://www.jedec.org/standards-documents/dictionary/terms/totem-pole-output ---snip---- The term "totem-pole output", as commonly used, does not include three-state outputs. ---snip---- [4] http://piclist.com/techref/postbot.asp?by=thread&id=%5BEE%5D+Push-pull+vs+totem+pole&w=body&tgt=post ---snip---- CMOS outputs are generally push-pull. They have a P-channel FET above and an N-channel fet below. Both are in digital model (on/off). 'Totem-pole' will never apply to a CMOS output. ---snip---- [5] http://www.ti.com/lit/ml/slua618a/slua618a.pdf ("Fundamentals of MOSFET and IGBT Gate Driver Circuits") ---snip---- The MOSFET equivalent of the bipolar totempole driver is pictured in Figure 11. [..] Unfortunately, this circuit has several drawbacks compared to the bipolar version that explain that it is very rarely implemented discretely. ---snip---- [6] https://www.raspberrypi.org/documentation/hardware/raspberrypi/gpio/gpio_pads_control.md [7] linux (v5.4-rc7) git grep -i quirk | wc -l 12047
Hi everyone, > > 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>; > > I don't understand why you have 3 values instead of 2. Because he sketches maximum flexibility here. Have a non-HS (default) value, one for HS200, and one for HS400: fixed-emmc-driver-type = <[default] [HS200] [HS400]>; > I would just use -1 if you want to ignore an entry. If that's the common '-1' sounds good to me, too. > case, then I'd stick with what you originally proposed. If rare, then I > think an array is the better route. What I have seen so far: setting drive strength alone is more on the rare side. Setting specific values for default and HS200/400 seems even more rare to me. With this patchset, it is the first time I hear about it. Yet, my experience might be a bit limited, maybe others (Hi Ulf! ;)) can add their views, too? Regards, Wolfram
Hi Eugeniu, On Mon, Nov 11, 2019 at 11:25 PM Eugeniu Rosca <erosca@de.adit-jv.com> wrote: > This matches my view with below amendments: > - Your passage seems to describe a single-duplex communication (one end > is always a sender and the other one is always a receiver). Since the > CMD and DAT[0-7] eMMC lines are bidirectional (carrying half-duplex > data transfers), this is what seems to justify the > "drive(r) strength/type" feature/setting to be present on both host > controller and eMMC device ends (which does happen in real life). > - I am unsure whether to endorse the "totempole output" as being the > usual foundation for how "drive strength" is really implemented in > the modern CMOS ICs, based on the following: > - All eMMC Jedec specs mention "push-pull" for CMD/DAT[0-7] > - All eMMC device datasheets I could find reference "push pull" > and none mentions "totem pole" for CMD/DAT[0-7] > - The "totem pole" topology seems to originate from and be much more > popular in the TTL/BJT world, where it tries to harness the > symmetry of two NPN transistors, replacing the PNP-NPN pair used > in the bipolar "push-pull" configuration [1-2]. > - Jedec calls totem-pole "a bipolar output" (i.e. TTL/BJT) [3] > - Jedec claims [3] that "vanilla" tottempole doesn't support > tristate/hi-Z outputs, making it impossible to connect several such > circuits in parallel, while we assume the latter to be a hard > prerequisite for sourcing programmable amounts of current. > - Some users say that "CMOS outputs are generally push-pull" [4] > - TI states [5] that the "MOSFET equivalent of the bipolar totempole > driver [..] is rarely implemented" > > Abstracting from the above, I agree that a programmable "drive strength" > is likely achieved by connecting several tristate-capable output > circuits in a "wired OR", as exemplified for Raspberry Pi in [6]. OK that's established. I am sorry for using the TTL "totempole" term here, it is out of place for CMOS indeed. Very nice detailing and references, I read them all :) > > Usually selection of drive strength is done with the pin control > > framework, so this would need to be backed by code (not in this > > patch set) that select pin control states that reconfigure the > > SoC pad drivers to use the requested strength. > > That's true. In the same context of overcoming HS400 issues, our SoC > vendor suggested adjusting the "drive-strength" binding, added in: > - 7db9af4b6e41be ("pinctrl: add function to parse generic pinconfig > properties from a dt node") > - 3caa7d8c3f03ad ("pinctrl: sh-pfc: Add drive strength support") OK so the pin controller will act as back-end for this and the drivers are expected to use that. I suppose you are defining new HSxxx-specific pin control states for them? I suppose it would be good to see how it works end-to-end. (But fine, I get it so far.) > > Alternatively, the (e)MMC block would implement this control > > directly, but I doubt it. > > There _is_ a "drive strength" specific to eMMC device and the > justification for it to exist has been made above. > > According to JESD84-B50.1 spec, the host controller is able to find > the "drive strength" values supported by a particular eMMC device by > reading the DRIVER_STRENGTH field of the Extended CSD. The host then > may (if needed), make use of this value to set the "Driver Strength" > parameter in the HS_TIMING field of the Extended CSD register. So the operating system reads the ext CSD and uses that information to set the drive strength using pin control in the Linux case. What is the unit of this driver strength field in the ext CSD? And consequently this: + fixed-emmc-driver-type-hs200: + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + - minimum: 0 + - maximum: 4 + description: + Same as "fixed-emmc-driver-type", but specific to HS200 mode. + If defined, overrides "fixed-emmc-driver-type" in HS200 mode. This thing that is 0,1,2,3,4, what unit is that? If we established it is the number of push-pull stages OR:ed together we should document it. Since it is supposed to be used with different host controllers, it certainly cannot be unitless or "vendor specific" since you have two sides to it, the card ext CSD and this, and then the pin controller side. (I'm not a standards person buy certainly JEDEC must have thought of that?) > Essentially, current series gives the host controller a chance to limit > the drive strength value written to HS_TIMING, if eMMC vendor decides > that some of the values advertised in DRIVER_STRENGTH are forbidden > or should be avoided in a specific bus speed mode (HS200/HS400). OK this text should really be in the commit message and the bindings because this isn't clear from context. It confused me so it will confuse others. I still don't quite get it, sorry if I'm dumb :/ Do you mean that the eMMC advertise some drive strengths in the ext CSD and the device tree properties are there to mitigate or override these and disallow them because of limitations in the host controller or associated electronics? That sounds more like something the system integrator/manufacturer decide than the eMMC vendor. Or it it a bug in the ext CSD so that the eMMC advertise strengths it shouldn't? Then we should use a quirk for the card ID rather than a DT property. > As explained above, this series allows to customize the eMMC-specific > drive strength. The eMMC vendor did not ask to make the SoC-side > drive strength dependent on bus speed mode and that was not needed in > the testing performed by the customer. I think I understand this now! Some nice details above makes it clear what these values are for. > > So to me it seems these DT properties are just introduced to > > hammer down a certain usecase instead of letting the code with the > > help of DT speed capabilities flags determine what speed is to be used > > and select the appropriate drive strength. > > Does this mean that the "fixed-emmc-driver-type" binding which > pre-exists my series falls under the same sentence? Or is this only > when customizing Wolfram's binding to HS200/HS400 bus speed mode? Now that it's clear that this is to restrict the drive strengths used I understand it better! > Note that there is no other objective in this series but to allow Linux > to run on hardware which doesn't strictly follow its specification [7]. > If you have any alternative ideas of how to follow the eMMC vendor's > recommendation quoted in the description of this patch, I will happily > review those. I'm a bit confused. This "recommendation" sounds like some errata actually. If the case is that eMMC vendor has recommended certain stuff in the ext CSD and you need to augment that with the device tree config, that sounds like the wrong approach. It is a bug in that eMMCs ext CSD is it not? Why can't we use code for this and just add a per-card quirk instead if there are errors in the drive strengths recommended in the ext CSD? Like the other stuff in drivers/mmc/core/quirks.h. That makes the same card work on any other host without any device tree special properties, hopefully. Yours, Linus Walleij
On Tue, Nov 12, 2019 at 10:19 PM Wolfram Sang <wsa@the-dreams.de> wrote: > What I have seen so far: setting drive strength alone is more on the > rare side. Setting specific values for default and HS200/400 seems even > more rare to me. With this patchset, it is the first time I hear about > it. Like I wrote to Eugeniu this sounds like some kind of errata for the eMMC ext CSD and should likely be a card quirk rather than some generic device tree properties. I might be wrong, we'll hash it out. Yours, Linus Walleij
[...] > > > > > > 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>; > > > > I don't understand why you have 3 values instead of 2. > > Because he sketches maximum flexibility here. Have a non-HS (default) > value, one for HS200, and one for HS400: > > fixed-emmc-driver-type = <[default] [HS200] [HS400]>; > > > I would just use -1 if you want to ignore an entry. If that's the common > > '-1' sounds good to me, too. > > > case, then I'd stick with what you originally proposed. If rare, then I > > think an array is the better route. > > What I have seen so far: setting drive strength alone is more on the > rare side. Setting specific values for default and HS200/400 seems even > more rare to me. With this patchset, it is the first time I hear about > it. > > Yet, my experience might be a bit limited, maybe others (Hi Ulf! ;)) can add > their views, too? My experience in this field is quite limited. No input from me, sorry. Kind regards Uffe
diff --git a/Documentation/devicetree/bindings/mmc/mmc-controller.yaml b/Documentation/devicetree/bindings/mmc/mmc-controller.yaml index 080754e0ef35..1c64b14f91a3 100644 --- a/Documentation/devicetree/bindings/mmc/mmc-controller.yaml +++ b/Documentation/devicetree/bindings/mmc/mmc-controller.yaml @@ -248,6 +248,24 @@ properties: the driver type as specified in the eMMC specification (table 206 in spec version 5.1) + fixed-emmc-driver-type-hs200: + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + - minimum: 0 + - maximum: 4 + description: + Same as "fixed-emmc-driver-type", but specific to HS200 mode. + If defined, overrides "fixed-emmc-driver-type" in HS200 mode. + + fixed-emmc-driver-type-hs400: + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + - minimum: 0 + - maximum: 4 + description: + Same as "fixed-emmc-driver-type", but specific to HS400 mode. + If defined, overrides "fixed-emmc-driver-type" in HS400 mode. + post-power-on-delay-ms: allOf: - $ref: /schemas/types.yaml#/definitions/uint32 @@ -336,6 +354,8 @@ patternProperties: dependencies: cd-debounce-delay-ms: [ cd-gpios ] fixed-emmc-driver-type: [ non-removable ] + fixed-emmc-driver-type-hs200: [ non-removable ] + fixed-emmc-driver-type-hs400: [ non-removable ] examples: - |
A certain eMMC manufacturer provided below requirement: ---snip--- Use "drive strength" value of 4 or 1 for HS400 or 0 for HS200. ---snip--- 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 For more details about eMMC I/O driver strength types, see Jedec spec. Keep "fixed-emmc-driver-type" in place for backward compatibility. [1] commit 6186d06c519e21 ("mmc: parse new binding for eMMC fixed driver type") Cc: Wolfram Sang <wsa+renesas@sang-engineering.com> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> --- .../bindings/mmc/mmc-controller.yaml | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+)