Introduce a new optional device tree property `max-sd-hs-frequency` to
limit the maximum frequency (in Hz) used for SD cards operating in
High-Speed (HS) mode.
This property is useful for platforms with vendor-specific hardware
constraints, such as the presence of a level shifter that cannot
reliably support the default 50 MHz HS frequency. It allows the host
driver to cap the HS mode frequency accordingly.
Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com>
---
.../devicetree/bindings/mmc/mmc-controller-common.yaml | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml b/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml
index 9a7235439759..1976f5f8c401 100644
--- a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml
+++ b/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml
@@ -93,6 +93,16 @@ properties:
minimum: 400000
maximum: 384000000
+ max-sd-hs-frequency:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: |
+ Maximum frequency (in Hz) to be used for SD cards operating in
+ High-Speed (HS) mode. This is useful for platforms with vendor-specific
+ limitations, such as the presence of a level shifter that cannot support
+ the default 50 MHz HS frequency or other.
+ minimum: 400000
+ maximum: 50000000
+
disable-wp:
$ref: /schemas/types.yaml#/definitions/flag
description:
--
2.34.1On 18/06/2025 09:28, Sarthak Garg wrote: > Introduce a new optional device tree property `max-sd-hs-frequency` to > limit the maximum frequency (in Hz) used for SD cards operating in > High-Speed (HS) mode. > > This property is useful for platforms with vendor-specific hardware > constraints, such as the presence of a level shifter that cannot > reliably support the default 50 MHz HS frequency. It allows the host > driver to cap the HS mode frequency accordingly. > > Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com> > --- > .../devicetree/bindings/mmc/mmc-controller-common.yaml | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml b/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml > index 9a7235439759..1976f5f8c401 100644 > --- a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml > +++ b/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml > @@ -93,6 +93,16 @@ properties: > minimum: 400000 > maximum: 384000000 > > + max-sd-hs-frequency: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: | > + Maximum frequency (in Hz) to be used for SD cards operating in > + High-Speed (HS) mode. This is useful for platforms with vendor-specific > + limitations, such as the presence of a level shifter that cannot support > + the default 50 MHz HS frequency or other. > + minimum: 400000 > + maximum: 50000000 This might be fine, but your DTS suggests clearly this is SoC compatible deducible, which I already said at v1. So now you send v3 which is the same as v1, so you get the same comments. Best regards, Krzysztof
On 6/18/25 9:43 AM, Krzysztof Kozlowski wrote: > On 18/06/2025 09:28, Sarthak Garg wrote: >> Introduce a new optional device tree property `max-sd-hs-frequency` to >> limit the maximum frequency (in Hz) used for SD cards operating in >> High-Speed (HS) mode. >> >> This property is useful for platforms with vendor-specific hardware >> constraints, such as the presence of a level shifter that cannot >> reliably support the default 50 MHz HS frequency. It allows the host >> driver to cap the HS mode frequency accordingly. >> >> Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com> >> --- >> .../devicetree/bindings/mmc/mmc-controller-common.yaml | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml b/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml >> index 9a7235439759..1976f5f8c401 100644 >> --- a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml >> +++ b/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml >> @@ -93,6 +93,16 @@ properties: >> minimum: 400000 >> maximum: 384000000 >> >> + max-sd-hs-frequency: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: | >> + Maximum frequency (in Hz) to be used for SD cards operating in >> + High-Speed (HS) mode. This is useful for platforms with vendor-specific >> + limitations, such as the presence of a level shifter that cannot support >> + the default 50 MHz HS frequency or other. >> + minimum: 400000 >> + maximum: 50000000 > > This might be fine, but your DTS suggests clearly this is SoC compatible > deducible, which I already said at v1. I don't understand why you're rejecting a common solution to a problem that surely exists outside this one specific chip from one specific vendor, which may be caused by a multitude of design choices, including erratic board (not SoC) electrical design Konrad
On 21/06/2025 12:20, Konrad Dybcio wrote: > On 6/18/25 9:43 AM, Krzysztof Kozlowski wrote: >> On 18/06/2025 09:28, Sarthak Garg wrote: >>> Introduce a new optional device tree property `max-sd-hs-frequency` to >>> limit the maximum frequency (in Hz) used for SD cards operating in >>> High-Speed (HS) mode. >>> >>> This property is useful for platforms with vendor-specific hardware >>> constraints, such as the presence of a level shifter that cannot >>> reliably support the default 50 MHz HS frequency. It allows the host >>> driver to cap the HS mode frequency accordingly. >>> >>> Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com> >>> --- >>> .../devicetree/bindings/mmc/mmc-controller-common.yaml | 10 ++++++++++ >>> 1 file changed, 10 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml b/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml >>> index 9a7235439759..1976f5f8c401 100644 >>> --- a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml >>> +++ b/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml >>> @@ -93,6 +93,16 @@ properties: >>> minimum: 400000 >>> maximum: 384000000 >>> >>> + max-sd-hs-frequency: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + description: | >>> + Maximum frequency (in Hz) to be used for SD cards operating in >>> + High-Speed (HS) mode. This is useful for platforms with vendor-specific >>> + limitations, such as the presence of a level shifter that cannot support >>> + the default 50 MHz HS frequency or other. >>> + minimum: 400000 >>> + maximum: 50000000 >> >> This might be fine, but your DTS suggests clearly this is SoC compatible >> deducible, which I already said at v1. > > I don't understand why you're rejecting a common solution to a problem > that surely exists outside this one specific chip from one specific > vendor, which may be caused by a multitude of design choices, including > erratic board (not SoC) electrical design No one brought any arguments so far that common solution is needed. The only argument provided - sm8550 - is showing this is soc design. I don't reject common solution. I provided review at v1 to which no one responded, no one argued, no one provided other arguments. Best regards, Krzysztof
On 6/22/25 11:48 AM, Krzysztof Kozlowski wrote: > On 21/06/2025 12:20, Konrad Dybcio wrote: >> On 6/18/25 9:43 AM, Krzysztof Kozlowski wrote: >>> On 18/06/2025 09:28, Sarthak Garg wrote: >>>> Introduce a new optional device tree property `max-sd-hs-frequency` to >>>> limit the maximum frequency (in Hz) used for SD cards operating in >>>> High-Speed (HS) mode. >>>> >>>> This property is useful for platforms with vendor-specific hardware >>>> constraints, such as the presence of a level shifter that cannot >>>> reliably support the default 50 MHz HS frequency. It allows the host >>>> driver to cap the HS mode frequency accordingly. >>>> >>>> Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com> >>>> --- >>>> .../devicetree/bindings/mmc/mmc-controller-common.yaml | 10 ++++++++++ >>>> 1 file changed, 10 insertions(+) >>>> >>>> diff --git a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml b/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml >>>> index 9a7235439759..1976f5f8c401 100644 >>>> --- a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml >>>> +++ b/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml >>>> @@ -93,6 +93,16 @@ properties: >>>> minimum: 400000 >>>> maximum: 384000000 >>>> >>>> + max-sd-hs-frequency: >>>> + $ref: /schemas/types.yaml#/definitions/uint32 >>>> + description: | >>>> + Maximum frequency (in Hz) to be used for SD cards operating in >>>> + High-Speed (HS) mode. This is useful for platforms with vendor-specific >>>> + limitations, such as the presence of a level shifter that cannot support >>>> + the default 50 MHz HS frequency or other. >>>> + minimum: 400000 >>>> + maximum: 50000000 >>> >>> This might be fine, but your DTS suggests clearly this is SoC compatible >>> deducible, which I already said at v1. >> >> I don't understand why you're rejecting a common solution to a problem >> that surely exists outside this one specific chip from one specific >> vendor, which may be caused by a multitude of design choices, including >> erratic board (not SoC) electrical design > > No one brought any arguments so far that common solution is needed. The > only argument provided - sm8550 - is showing this is soc design. > > I don't reject common solution. I provided review at v1 to which no one > responded, no one argued, no one provided other arguments. Okay, so the specific problem that causes this observable limitation exists on SM8550 and at least one more platform which is not upstream today. It can be caused by various electrical issues, in our specific case by something internal to the SoC (but external factors may apply too) Looking at the docs, a number of platforms have various limitations with regards to frequency at specific speed-modes, some of which seem to be handled implicitly by rounding in the clock framework's round/set_rate(). I can very easily imagine there are either boards or platforms in the wild, where the speed must be limited for various reasons, maybe some of them currently don't advertise it (like sm8550 on next/master) to hide that Konrad
On 23/06/2025 14:08, Konrad Dybcio wrote: >>>> >>>> This might be fine, but your DTS suggests clearly this is SoC compatible >>>> deducible, which I already said at v1. >>> >>> I don't understand why you're rejecting a common solution to a problem >>> that surely exists outside this one specific chip from one specific >>> vendor, which may be caused by a multitude of design choices, including >>> erratic board (not SoC) electrical design >> >> No one brought any arguments so far that common solution is needed. The >> only argument provided - sm8550 - is showing this is soc design. >> >> I don't reject common solution. I provided review at v1 to which no one >> responded, no one argued, no one provided other arguments. > > Okay, so the specific problem that causes this observable limitation > exists on SM8550 and at least one more platform which is not upstream > today. It can be caused by various electrical issues, in our specific > case by something internal to the SoC (but external factors may apply > too) > > Looking at the docs, a number of platforms have various limitations > with regards to frequency at specific speed-modes, some of which seem > to be handled implicitly by rounding in the clock framework's > round/set_rate(). > > I can very easily imagine there are either boards or platforms in the > wild, where the speed must be limited for various reasons, maybe some > of them currently don't advertise it (like sm8550 on next/master) to > hide that But there are no such now. The only argument (fact) provided in this patchset is: this is issue specific to SM8550 SoC, not the board. See last patch. Therefore this is compatible-deducible and this makes property without any upstream user. Best regards, Krzysztof
On 6/23/25 2:16 PM, Krzysztof Kozlowski wrote: > On 23/06/2025 14:08, Konrad Dybcio wrote: >>>>> >>>>> This might be fine, but your DTS suggests clearly this is SoC compatible >>>>> deducible, which I already said at v1. >>>> >>>> I don't understand why you're rejecting a common solution to a problem >>>> that surely exists outside this one specific chip from one specific >>>> vendor, which may be caused by a multitude of design choices, including >>>> erratic board (not SoC) electrical design >>> >>> No one brought any arguments so far that common solution is needed. The >>> only argument provided - sm8550 - is showing this is soc design. >>> >>> I don't reject common solution. I provided review at v1 to which no one >>> responded, no one argued, no one provided other arguments. >> >> Okay, so the specific problem that causes this observable limitation >> exists on SM8550 and at least one more platform which is not upstream >> today. It can be caused by various electrical issues, in our specific >> case by something internal to the SoC (but external factors may apply >> too) >> >> Looking at the docs, a number of platforms have various limitations >> with regards to frequency at specific speed-modes, some of which seem >> to be handled implicitly by rounding in the clock framework's >> round/set_rate(). >> >> I can very easily imagine there are either boards or platforms in the >> wild, where the speed must be limited for various reasons, maybe some >> of them currently don't advertise it (like sm8550 on next/master) to >> hide that > > But there are no such now. The only argument (fact) provided in this > patchset is: this is issue specific to SM8550 SoC, not the board. See > last patch. Therefore this is compatible-deducible and this makes > property without any upstream user. When one appears, we will have to carry code to repeat what the property does, based on a specific compatible.. And all OS implementations will have to do the same, instead of parsing the explicit information Konrad
On 23/06/2025 14:31, Konrad Dybcio wrote: > On 6/23/25 2:16 PM, Krzysztof Kozlowski wrote: >> On 23/06/2025 14:08, Konrad Dybcio wrote: >>>>>> >>>>>> This might be fine, but your DTS suggests clearly this is SoC compatible >>>>>> deducible, which I already said at v1. >>>>> >>>>> I don't understand why you're rejecting a common solution to a problem >>>>> that surely exists outside this one specific chip from one specific >>>>> vendor, which may be caused by a multitude of design choices, including >>>>> erratic board (not SoC) electrical design >>>> >>>> No one brought any arguments so far that common solution is needed. The >>>> only argument provided - sm8550 - is showing this is soc design. >>>> >>>> I don't reject common solution. I provided review at v1 to which no one >>>> responded, no one argued, no one provided other arguments. >>> >>> Okay, so the specific problem that causes this observable limitation >>> exists on SM8550 and at least one more platform which is not upstream >>> today. It can be caused by various electrical issues, in our specific >>> case by something internal to the SoC (but external factors may apply >>> too) >>> >>> Looking at the docs, a number of platforms have various limitations >>> with regards to frequency at specific speed-modes, some of which seem >>> to be handled implicitly by rounding in the clock framework's >>> round/set_rate(). >>> >>> I can very easily imagine there are either boards or platforms in the >>> wild, where the speed must be limited for various reasons, maybe some >>> of them currently don't advertise it (like sm8550 on next/master) to >>> hide that >> >> But there are no such now. The only argument (fact) provided in this >> patchset is: this is issue specific to SM8550 SoC, not the board. See >> last patch. Therefore this is compatible-deducible and this makes >> property without any upstream user. > > When one appears, we will have to carry code to repeat what the property > does, based on a specific compatible.. And all OS implementations will > have to do the same, instead of parsing the explicit information Adding new property in such case will be trivial and simple, unlike having to maintain unused ABI. And it will be unused, because last patch DTS should be rejected on that basis: adding redundant properties which are already defined by the compatible. Best regards, Krzysztof
On 24-Jun-25 08:06, Krzysztof Kozlowski wrote: > On 23/06/2025 14:31, Konrad Dybcio wrote: >> On 6/23/25 2:16 PM, Krzysztof Kozlowski wrote: >>> On 23/06/2025 14:08, Konrad Dybcio wrote: >>>>>>> >>>>>>> This might be fine, but your DTS suggests clearly this is SoC compatible >>>>>>> deducible, which I already said at v1. >>>>>> >>>>>> I don't understand why you're rejecting a common solution to a problem >>>>>> that surely exists outside this one specific chip from one specific >>>>>> vendor, which may be caused by a multitude of design choices, including >>>>>> erratic board (not SoC) electrical design >>>>> >>>>> No one brought any arguments so far that common solution is needed. The >>>>> only argument provided - sm8550 - is showing this is soc design. >>>>> >>>>> I don't reject common solution. I provided review at v1 to which no one >>>>> responded, no one argued, no one provided other arguments. >>>> >>>> Okay, so the specific problem that causes this observable limitation >>>> exists on SM8550 and at least one more platform which is not upstream >>>> today. It can be caused by various electrical issues, in our specific >>>> case by something internal to the SoC (but external factors may apply >>>> too) >>>> >>>> Looking at the docs, a number of platforms have various limitations >>>> with regards to frequency at specific speed-modes, some of which seem >>>> to be handled implicitly by rounding in the clock framework's >>>> round/set_rate(). >>>> >>>> I can very easily imagine there are either boards or platforms in the >>>> wild, where the speed must be limited for various reasons, maybe some >>>> of them currently don't advertise it (like sm8550 on next/master) to >>>> hide that >>> >>> But there are no such now. The only argument (fact) provided in this >>> patchset is: this is issue specific to SM8550 SoC, not the board. See >>> last patch. Therefore this is compatible-deducible and this makes >>> property without any upstream user. >> >> When one appears, we will have to carry code to repeat what the property >> does, based on a specific compatible.. And all OS implementations will >> have to do the same, instead of parsing the explicit information > > Adding new property in such case will be trivial and simple, unlike > having to maintain unused ABI. > > And it will be unused, because last patch DTS should be rejected on that > basis: adding redundant properties which are already defined by the > compatible. Got some more fresh information.. This apparently *does* vary across boards, as there is a recommended hardware workaround to this rate limitation (requiring an external clock source, which is up to the OEM to implement or not) Konrad
On 01/07/2025 11:04, Konrad Dybcio wrote: >>>>> >>>>> Looking at the docs, a number of platforms have various limitations >>>>> with regards to frequency at specific speed-modes, some of which seem >>>>> to be handled implicitly by rounding in the clock framework's >>>>> round/set_rate(). >>>>> >>>>> I can very easily imagine there are either boards or platforms in the >>>>> wild, where the speed must be limited for various reasons, maybe some >>>>> of them currently don't advertise it (like sm8550 on next/master) to >>>>> hide that >>>> >>>> But there are no such now. The only argument (fact) provided in this >>>> patchset is: this is issue specific to SM8550 SoC, not the board. See >>>> last patch. Therefore this is compatible-deducible and this makes >>>> property without any upstream user. >>> >>> When one appears, we will have to carry code to repeat what the property >>> does, based on a specific compatible.. And all OS implementations will >>> have to do the same, instead of parsing the explicit information >> >> Adding new property in such case will be trivial and simple, unlike >> having to maintain unused ABI. >> >> And it will be unused, because last patch DTS should be rejected on that >> basis: adding redundant properties which are already defined by the >> compatible. > > Got some more fresh information.. This apparently *does* vary across > boards, as there is a recommended hardware workaround to this rate > limitation (requiring an external clock source, which is up to the > OEM to implement or not) This should be clearly explained in commit msg and the DTS patch re-written because it seems it is not a property of the SoC. I mean, really, that last patch here makes entire discussion pointless, because till it is in the patchset is a proof this is a SoC level property. Best regards, Krzysztof
On 7/1/2025 3:00 PM, Krzysztof Kozlowski wrote: > On 01/07/2025 11:04, Konrad Dybcio wrote: >>>>>> >>>>>> Looking at the docs, a number of platforms have various limitations >>>>>> with regards to frequency at specific speed-modes, some of which seem >>>>>> to be handled implicitly by rounding in the clock framework's >>>>>> round/set_rate(). >>>>>> >>>>>> I can very easily imagine there are either boards or platforms in the >>>>>> wild, where the speed must be limited for various reasons, maybe some >>>>>> of them currently don't advertise it (like sm8550 on next/master) to >>>>>> hide that >>>>> >>>>> But there are no such now. The only argument (fact) provided in this >>>>> patchset is: this is issue specific to SM8550 SoC, not the board. See >>>>> last patch. Therefore this is compatible-deducible and this makes >>>>> property without any upstream user. >>>> >>>> When one appears, we will have to carry code to repeat what the property >>>> does, based on a specific compatible.. And all OS implementations will >>>> have to do the same, instead of parsing the explicit information >>> >>> Adding new property in such case will be trivial and simple, unlike >>> having to maintain unused ABI. >>> >>> And it will be unused, because last patch DTS should be rejected on that >>> basis: adding redundant properties which are already defined by the >>> compatible. >> >> Got some more fresh information.. This apparently *does* vary across >> boards, as there is a recommended hardware workaround to this rate >> limitation (requiring an external clock source, which is up to the >> OEM to implement or not) > > > This should be clearly explained in commit msg and the DTS patch > re-written because it seems it is not a property of the SoC. > > I mean, really, that last patch here makes entire discussion pointless, > because till it is in the patchset is a proof this is a SoC level property. > > Best regards, > Krzysztof Sure I'll update the commit message clearly mentioning it as board specific and update the DTS patch and have these changes in board specific dts files (for e.g sm8550-mtp.dts). As rightly stated above this configuration will vary across boards as well. Regards, Sarthak
On 6/18/2025 1:13 PM, Krzysztof Kozlowski wrote: > On 18/06/2025 09:28, Sarthak Garg wrote: >> Introduce a new optional device tree property `max-sd-hs-frequency` to >> limit the maximum frequency (in Hz) used for SD cards operating in >> High-Speed (HS) mode. >> >> This property is useful for platforms with vendor-specific hardware >> constraints, such as the presence of a level shifter that cannot >> reliably support the default 50 MHz HS frequency. It allows the host >> driver to cap the HS mode frequency accordingly. >> >> Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com> >> --- >> .../devicetree/bindings/mmc/mmc-controller-common.yaml | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml b/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml >> index 9a7235439759..1976f5f8c401 100644 >> --- a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml >> +++ b/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml >> @@ -93,6 +93,16 @@ properties: >> minimum: 400000 >> maximum: 384000000 >> >> + max-sd-hs-frequency: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: | >> + Maximum frequency (in Hz) to be used for SD cards operating in >> + High-Speed (HS) mode. This is useful for platforms with vendor-specific >> + limitations, such as the presence of a level shifter that cannot support >> + the default 50 MHz HS frequency or other. >> + minimum: 400000 >> + maximum: 50000000 > > This might be fine, but your DTS suggests clearly this is SoC compatible > deducible, which I already said at v1. > > So now you send v3 which is the same as v1, so you get the same comments. > > Best regards, > Krzysztof Introducing this flag no longer becomes SoC compatible because as per discussions in V2 patchset with Ulf and Konrad this new property can be used by any vendor who wants to limit the HS mode frequency due to any reason. Thats why moved to this generic approach again in V3 as compared to compatible based approach in V2.
On 18/06/2025 10:38, Sarthak Garg wrote: > > > On 6/18/2025 1:13 PM, Krzysztof Kozlowski wrote: >> On 18/06/2025 09:28, Sarthak Garg wrote: >>> Introduce a new optional device tree property `max-sd-hs-frequency` to >>> limit the maximum frequency (in Hz) used for SD cards operating in >>> High-Speed (HS) mode. >>> >>> This property is useful for platforms with vendor-specific hardware >>> constraints, such as the presence of a level shifter that cannot >>> reliably support the default 50 MHz HS frequency. It allows the host >>> driver to cap the HS mode frequency accordingly. >>> >>> Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com> >>> --- >>> .../devicetree/bindings/mmc/mmc-controller-common.yaml | 10 ++++++++++ >>> 1 file changed, 10 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml b/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml >>> index 9a7235439759..1976f5f8c401 100644 >>> --- a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml >>> +++ b/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml >>> @@ -93,6 +93,16 @@ properties: >>> minimum: 400000 >>> maximum: 384000000 >>> >>> + max-sd-hs-frequency: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + description: | >>> + Maximum frequency (in Hz) to be used for SD cards operating in >>> + High-Speed (HS) mode. This is useful for platforms with vendor-specific >>> + limitations, such as the presence of a level shifter that cannot support >>> + the default 50 MHz HS frequency or other. >>> + minimum: 400000 >>> + maximum: 50000000 >> >> This might be fine, but your DTS suggests clearly this is SoC compatible >> deducible, which I already said at v1. >> >> So now you send v3 which is the same as v1, so you get the same comments. >> >> Best regards, >> Krzysztof > > Introducing this flag no longer becomes SoC compatible because as per > discussions in V2 patchset with Ulf and Konrad this new property can be > used by any vendor who wants to limit the HS mode frequency due to any > reason. Thats why moved to this generic approach again in V3 as compared > to compatible based approach in V2. The are no arguments provided in favor, so my review from v1 stays. You get the same comments. Best regards, Krzysztof
On 6/18/2025 2:38 PM, Krzysztof Kozlowski wrote: > On 18/06/2025 10:38, Sarthak Garg wrote: >> >> >> On 6/18/2025 1:13 PM, Krzysztof Kozlowski wrote: >>> On 18/06/2025 09:28, Sarthak Garg wrote: >>>> Introduce a new optional device tree property `max-sd-hs-frequency` to >>>> limit the maximum frequency (in Hz) used for SD cards operating in >>>> High-Speed (HS) mode. >>>> >>>> This property is useful for platforms with vendor-specific hardware >>>> constraints, such as the presence of a level shifter that cannot >>>> reliably support the default 50 MHz HS frequency. It allows the host >>>> driver to cap the HS mode frequency accordingly. >>>> >>>> Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com> >>>> --- >>>> .../devicetree/bindings/mmc/mmc-controller-common.yaml | 10 ++++++++++ >>>> 1 file changed, 10 insertions(+) >>>> >>>> diff --git a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml b/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml >>>> index 9a7235439759..1976f5f8c401 100644 >>>> --- a/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml >>>> +++ b/Documentation/devicetree/bindings/mmc/mmc-controller-common.yaml >>>> @@ -93,6 +93,16 @@ properties: >>>> minimum: 400000 >>>> maximum: 384000000 >>>> >>>> + max-sd-hs-frequency: >>>> + $ref: /schemas/types.yaml#/definitions/uint32 >>>> + description: | >>>> + Maximum frequency (in Hz) to be used for SD cards operating in >>>> + High-Speed (HS) mode. This is useful for platforms with vendor-specific >>>> + limitations, such as the presence of a level shifter that cannot support >>>> + the default 50 MHz HS frequency or other. >>>> + minimum: 400000 >>>> + maximum: 50000000 >>> >>> This might be fine, but your DTS suggests clearly this is SoC compatible >>> deducible, which I already said at v1. >>> >>> So now you send v3 which is the same as v1, so you get the same comments. >>> >>> Best regards, >>> Krzysztof >> >> Introducing this flag no longer becomes SoC compatible because as per >> discussions in V2 patchset with Ulf and Konrad this new property can be >> used by any vendor who wants to limit the HS mode frequency due to any >> reason. Thats why moved to this generic approach again in V3 as compared >> to compatible based approach in V2. > > The are no arguments provided in favor, so my review from v1 stays. You > get the same comments. > > Best regards, > Krzysztof @Ulf, @Konrad — since you previously supported the idea of a generic property for HS frequency limitation, could you please share your thoughts on whether this still seems like a valid approach? Best regards, Sarthak
© 2016 - 2026 Red Hat, Inc.