dt-bindings: mmc: sdhci-msm: Add CQE reg map
Subsystems (3)
PATCH 1/1 dt-bindings: mmc: sdhci-msm: Add CQE reg map
CQE feature has been enabled on sdhci-msm. Add CQE reg map that needs to be supplied for supporting CQE feature.
--- Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 5 +++++ 1 file changed, 5 insertions(+)
Documentation/devicetree/bindings/mmc/sdhci-msm.txt +5 -0
@@ -27,6 +27,11 @@ Required properties: - reg: Base address and length of the register in the following order: - Host controller register map (required) - SD Core register map (required for msm-v4 and below) + - CQE register map (Optional, needed only for eMMC and msm-v4.2 above) +- reg-names: When CQE register map is supplied, below reg-names are required + - "hc_mem" for Host controller register map + - "core_mem" for SD cpre regoster map + - "cqhci_mem" for CQE register map - interrupts: Should contain an interrupt-specifiers for the interrupts: - Host controller interrupt (required) - pinctrl-names: Should contain only one value - "default".
Signature · 1 line
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Hi,
Quoted 15 lines · Veerabhadrarao Badiganti
CQE feature has been enabled on sdhci-msm. Add CQE reg map that needs to be supplied for supporting CQE feature.
Change-Id I788c4bd5b7cbca16bc1030a410cc5550ed7204e1Signed-off-by Veerabhadrarao Badiganti <[email protected]>--- Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 5 +++++ 1 file changed, 5 insertions(+)Documentation/devicetree/bindings/mmc/sdhci-msm.txt +0
-0
Documentation/devicetree/bindings/mmc/sdhci-msm.txt Required properties: · line 30
Quoted 4 lines
Documentation/devicetree/bindings/mmc/sdhci-msm.txt +1
-0@@ -27,6 +27,11 @@ Required properties: - reg: Base address and length of the register in the following order: - Host controller register map (required) - SD Core register map (required for msm-v4 and below) + - CQE register map (Optional, needed only for eMMC and msm-v4.2 above)
I did a quick search and it appears that SD cards implementing 6.0 of the spec can also use CQE. Is that correct? If so, maybe remove the part about "eMMC"?
Maybe also change "needed" to "useful" to make it clear that this entry isn't actually required for all msm-v4.2 controllers?
Documentation/devicetree/bindings/mmc/sdhci-msm.txt Required properties: · line 33
Quoted 3 lines
Documentation/devicetree/bindings/mmc/sdhci-msm.txt +3
-0+- reg-names: When CQE register map is supplied, below reg-names are required + - "hc_mem" for Host controller register map + - "core_mem" for SD cpre regoster map
s/regoster/register
Documentation/devicetree/bindings/mmc/sdhci-msm.txt Required properties: · line 34
Quoted 1 line
Documentation/devicetree/bindings/mmc/sdhci-msm.txt +1
-0+ - "cqhci_mem" for CQE register map
I'm at least slightly confused. You say that reg-names are there only if CQE register map is supplied. ...and that requires 4.2 and above. ...but "core_mem" is only there on 4.0 and below. So there should never be a "core_mem" entry?
Trying to specify that sanely in free-form text seems like it's gonna be hard and not worth it. You should probably transition to yaml first?
I will also note that Rob isn't a huge fan of "reg-names". In a different conversation I think you mentioned you had a reason for having it. I guess just be prepared to defend yourself against Rob if you feel strongly about keeping reg-names.
-Doug
Quoted 13 lines · Doug Anderson
Hi,
Quoted 9 lines · Veerabhadrarao Badiganti
CQE feature has been enabled on sdhci-msm. Add CQE reg map that needs to be supplied for supporting CQE feature.
Change-Id I788c4bd5b7cbca16bc1030a410cc5550ed7204e1Signed-off-by Veerabhadrarao Badiganti <[email protected]>--- Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 5 +++++ 1 file changed, 5 insertions(+)
Quoted 12 lines
Documentation/devicetree/bindings/mmc/sdhci-msm.txt Required properties: · line 30
Quoted 9 lines
Documentation/devicetree/bindings/mmc/sdhci-msm.txt +1
-0@@ -27,6 +27,11 @@ Required properties: - reg: Base address and length of the register in the following order: - Host controller register map (required) - SD Core register map (required for msm-v4 and below) + - CQE register map (Optional, needed only for eMMC and msm-v4.2 above)I did a quick search and it appears that SD cards implementing 6.0 of the spec can also use CQE. Is that correct? If so, maybe remove the part about "eMMC"?
On qcom platforms, only SDHC instance meant for eMMC has the CQE support. So mentioned that its needed only for eMMC.
Quoted 3 lines
Maybe also change "needed" to "useful" to make it clear that this entry isn't actually required for all msm-v4.2 controllers?
sure.
Quoted 4 lines
Documentation/devicetree/bindings/mmc/sdhci-msm.txt Required properties: · line 33
Quoted 4 lines
Documentation/devicetree/bindings/mmc/sdhci-msm.txt +3
-0+- reg-names: When CQE register map is supplied, below reg-names are required + - "hc_mem" for Host controller register map + - "core_mem" for SD cpre regoster map
Quoted 8 lines
s/regoster/register
Documentation/devicetree/bindings/mmc/sdhci-msm.txt Required properties: · line 34
Quoted 1 line
Documentation/devicetree/bindings/mmc/sdhci-msm.txt +1
-0+ - "cqhci_mem" for CQE register mapI'm at least slightly confused. You say that reg-names are there only if CQE register map is supplied. ...and that requires 4.2 and above. ...but "
core_mem" is only there on 4.0 and below. So there should never be a "core_mem" entry?
core_mem is present till <v5.0cqhci_mem is present on >=v4.2
Say, for version v4.2 both are present; .... and for v5.0 only cqhci_mem is present.
Both hc reg-map and core reg-map are being accessed through index. So no need to list the reg names 'hc_mem' & 'core_mem' in general.
But coming to cqhci reg-map we can't access it with fixed index, since its index varies between 1/2 based on controller version.
So we are accessing it through reg-names. Since reg-names has to be associated with corresponding reg maps, other two reg-names (hc_mem & core_mem) also need to br listed when cqhci_mem is listed.
That is the reason, I mentioned it like these are needed only cqe reg map is supplied. If it is creating confusion, i will remove that statement.
Quoted 9 lines
Trying to specify that sanely in free-form text seems like it's gonna be hard and not worth it. You should probably transition to yaml first?
I will also note that Rob isn't a huge fan of "reg-names". In a different conversation I think you mentioned you had a reason for having it. I guess just be prepared to defend yourself against Rob if you feel strongly about keeping reg-names.
Sure. Its the same reason mentioned in above comment.
Quoted 2 lines
-Doug
Hi,
Quoted 28 lines · Veerabhadrarao Badiganti
Quoted 13 lines · Doug Anderson
Hi,
Quoted 9 lines · Veerabhadrarao Badiganti
CQE feature has been enabled on sdhci-msm. Add CQE reg map that needs to be supplied for supporting CQE feature.
Change-Id I788c4bd5b7cbca16bc1030a410cc5550ed7204e1Signed-off-by Veerabhadrarao Badiganti <[email protected]>--- Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 5 +++++ 1 file changed, 5 insertions(+)Quoted 12 lines
Documentation/devicetree/bindings/mmc/sdhci-msm.txt Required properties: · line 30
Quoted 9 lines
Documentation/devicetree/bindings/mmc/sdhci-msm.txt +1
-0@@ -27,6 +27,11 @@ Required properties: - reg: Base address and length of the register in the following order: - Host controller register map (required) - SD Core register map (required for msm-v4 and below) + - CQE register map (Optional, needed only for eMMC and msm-v4.2 above)I did a quick search and it appears that SD cards implementing 6.0 of the spec can also use CQE. Is that correct? If so, maybe remove the part about "eMMC"?
Quoted 2 lines
On qcom platforms, only SDHC instance meant for eMMC has the CQE support. So mentioned that its needed only for eMMC.
Ah, got it. Maybe mention this in the bindings? Like "Optional, CQE is only implemented on controllers meant for eMMC and version v4.2 and above"
Quoted 8 lines
Quoted 2 lines
Maybe also change "needed" to "useful" to make it clear that this entry isn't actually required for all msm-v4.2 controllers?
sure.
Quoted 5 lines
Documentation/devicetree/bindings/mmc/sdhci-msm.txt Required properties: · line 33
Quoted 4 lines
Documentation/devicetree/bindings/mmc/sdhci-msm.txt +3
-0+- reg-names: When CQE register map is supplied, below reg-names are required + - "hc_mem" for Host controller register map + - "core_mem" for SD cpre regoster maps/regoster/register
Oh, also s/cpre/core
Quoted 17 lines
Quoted 5 lines
Documentation/devicetree/bindings/mmc/sdhci-msm.txt Required properties: · line 34
Quoted 1 line
Documentation/devicetree/bindings/mmc/sdhci-msm.txt +1
-0+ - "cqhci_mem" for CQE register mapI'm at least slightly confused. You say that reg-names are there only if CQE register map is supplied. ...and that requires 4.2 and above. ...but "
core_mem" is only there on 4.0 and below. So there should never be a "core_mem" entry?
core_memis present till <v5.0cqhci_memis present on >=v4.2
Say, for version v4.2 both are present; .... and for v5.0 onlycqhci_memis present.Both hc reg-map and core reg-map are being accessed through index. So no need to list the reg names '
hc_mem' & 'core_mem' in general.But coming to cqhci reg-map we can't access it with fixed index, since its index varies between 1/2
based on controller version.
Quoted 8 lines
So we are accessing it through reg-names. Since reg-names has to be associated with corresponding
reg maps, other two reg-names (hc_mem&core_mem) also need to br listed whencqhci_memis listed.That is the reason, I mentioned it like these are needed only cqe reg map is supplied.
If it is creating confusion, i will remove that statement.
Ah. I think I got confused! When I saw "msm-v4 and below" in the description of "SD Core register map", I assumed that means that "v4.2" didn't have it. Maybe would be less confusing to change to:
- SD Core register map (required for controllers earlier than msm-v5)
Then I think what you have can be fine.
Quoted 10 lines
Quoted 9 lines
Trying to specify that sanely in free-form text seems like it's gonna be hard and not worth it. You should probably transition to yaml first?
I will also note that Rob isn't a huge fan of "reg-names". In a different conversation I think you mentioned you had a reason for having it. I guess just be prepared to defend yourself against Rob if you feel strongly about keeping reg-names.
Sure. Its the same reason mentioned in above comment.
OK. You've convinced me. It's still up to Rob but it seems like you have a good justification now that I understand it better.. ;-)
I think you could send out a v2 with the small wording changes and maybe it would be landable, but you'd want to follow up rather soon with the yaml conversion.
-Doug
PATCH v2 1/1 dt-bindings: mmc: sdhci-msm: Add CQE reg map
In reply to Veerabhadrarao BadigantiCQE feature has been enabled on sdhci-msm. Add CQE reg map that needs to be supplied for supporting CQE feature.
---
Changes since V1:
- Updated description for more clarity & Fixed typos.
--- Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/mmc/sdhci-msm.txt +7 -1
@@ -26,7 +26,13 @@ Required properties: - reg: Base address and length of the register in the following order: - Host controller register map (required) - - SD Core register map (required for msm-v4 and below) + - SD Core register map (required for controllers earlier than msm-v5) + - CQE register map (Optional, CQE support is present on SDHC instance meant + for eMMC and version v4.2 and above) +- reg-names: When CQE register map is supplied, below reg-names are required + - "hc_mem" for Host controller register map + - "core_mem" for SD core register map + - "cqhci_mem" for CQE register map - interrupts: Should contain an interrupt-specifiers for the interrupts: - Host controller interrupt (required) - pinctrl-names: Should contain only one value - "default".
Signature · 1 line
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Rob Herring
maintainerQuoted 18 lines · Veerabhadrarao Badiganti
CQE feature has been enabled on sdhci-msm. Add CQE reg map that needs to be supplied for supporting CQE feature.
Signed-off-by Veerabhadrarao Badiganti <[email protected]>---
Changes since V1:
- Updated description for more clarity & Fixed typos.--- Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)Documentation/devicetree/bindings/mmc/sdhci-msm.txt +0
-0@@ -26,7 +26,13 @@ Required properties:
Documentation/devicetree/bindings/mmc/sdhci-msm.txt Required properties: · line 35
Quoted 10 lines
Documentation/devicetree/bindings/mmc/sdhci-msm.txt +7
-1- reg: Base address and length of the register in the following order: - Host controller register map (required) - - SD Core register map (required for msm-v4 and below) + - SD Core register map (required for controllers earlier than msm-v5) + - CQE register map (Optional, CQE support is present on SDHC instance meant + for eMMC and version v4.2 and above) +- reg-names: When CQE register map is supplied, below reg-names are required + - "hc_mem" for Host controller register map + - "core_mem" for SD core register map + - "cqhci_mem" for CQE register map
'_mem' is redundant, so drop.
Quoted 6 lines
Documentation/devicetree/bindings/mmc/sdhci-msm.txt +0
-0- interrupts: Should contain an interrupt-specifiers for the interrupts: - Host controller interrupt (required) - pinctrl-names: Should contain only one value - "default".Signature · 1 line
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH v2 1/1 dt-bindings: mmc: sdhci-msm: Add CQE reg map
In reply to Veerabhadrarao BadigantiCQE feature has been enabled on sdhci-msm. Add CQE reg map and reg names that need to be supplied for supporting CQE feature.
---
Changes since V1:
- Dropped _mem suffix to reg names.
--- Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/mmc/sdhci-msm.txt +7 -1
@@ -26,7 +26,13 @@ Required properties: - reg: Base address and length of the register in the following order: - Host controller register map (required) - - SD Core register map (required for msm-v4 and below) + - SD Core register map (required for controllers earlier than msm-v5) + - CQE register map (Optional, CQE support is present on SDHC instance meant + for eMMC and version v4.2 and above) +- reg-names: When CQE register map is supplied, below reg-names are required + - "hc" for Host controller register map + - "core" for SD core register map + - "cqhci" for CQE register map - interrupts: Should contain an interrupt-specifiers for the interrupts: - Host controller interrupt (required) - pinctrl-names: Should contain only one value - "default".
Signature · 1 line
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
PATCH v3 1/1 dt-bindings: mmc: sdhci-msm: Add CQE reg map
In reply to Veerabhadrarao BadigantiCQE feature has been enabled on sdhci-msm. Add CQE reg map and reg names that need to be supplied for supporting CQE feature.
---
Changes since V2:
- Dropped _mem suffix to reg names.
Changes since V1:
- Updated description for more clarity & Fixed typos.
--- Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
Documentation/devicetree/bindings/mmc/sdhci-msm.txt +7 -1
@@ -26,7 +26,13 @@ Required properties: - reg: Base address and length of the register in the following order: - Host controller register map (required) - - SD Core register map (required for msm-v4 and below) + - SD Core register map (required for controllers earlier than msm-v5) + - CQE register map (Optional, CQE support is present on SDHC instance meant + for eMMC and version v4.2 and above) +- reg-names: When CQE register map is supplied, below reg-names are required + - "hc" for Host controller register map + - "core" for SD core register map + - "cqhci" for CQE register map - interrupts: Should contain an interrupt-specifiers for the interrupts: - Host controller interrupt (required) - pinctrl-names: Should contain only one value - "default".
Signature · 1 line
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Hi,
Quoted 9 lines · Veerabhadrarao Badiganti
CQE feature has been enabled on sdhci-msm. Add CQE reg map and reg names that need to be supplied for supporting CQE feature.
Signed-off-by Veerabhadrarao Badiganti <[email protected]>---
Changes since V2:
- Dropped_memsuffix to reg names.
Quoted 5 lines
Changes since V1:
- Updated description for more clarity & Fixed typos.--- Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
I assume you'll have a follow-up fixing the driver since commit a4080225f51d ("mmc: cqhci: support for command queue enabled host") refers to "cqhci_mem".
Also something to keep in mind for future patches (no action needed for this patch): most maintainers frown on making v2 of a patch "In-Reply-To" v1 of a patch. I notice that your v3 was in-reply-to v2 and v2 was in-reply-to v1. You probably don't want to do this. One such reference to people not liking it [1] specifically said "they should not be replies to old versions of that patch; otherwise the threading looks really weird and confusing."
[1] https://lore.kernel.org/r/CAJWu+oocs3T8orMNt6AmdVgWONzZg0vD=E8EdvzE9rOi_XatUw@mail.gmail.com
-Doug
Quoted 8 lines · Veerabhadrarao Badiganti
CQE feature has been enabled on sdhci-msm. Add CQE reg map and reg names that need to be supplied for supporting CQE feature.
Signed-off-by Veerabhadrarao Badiganti <[email protected]>---
Changes since V2:
- Dropped_memsuffix to reg names.
Quoted 6 lines
Changes since V1:
- Updated description for more clarity & Fixed typos.--- Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
Quoted 5 lines · Veerabhadrarao Badiganti
CQE feature has been enabled on sdhci-msm. Add CQE reg map and reg names that need to be supplied for supporting CQE feature.
Signed-off-by Veerabhadrarao Badiganti <[email protected]>
Applied for next, thanks!
Kind regards
Uffe
Quoted 31 lines
---
Changes since V2:
- Dropped_memsuffix to reg names.Changes since V1:
- Updated description for more clarity & Fixed typos.--- Documentation/devicetree/bindings/mmc/sdhci-msm.txt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)Documentation/devicetree/bindings/mmc/sdhci-msm.txt +7
-2@@ -26,7 +26,13 @@ Required properties: - reg: Base address and length of the register in the following order: - Host controller register map (required) - - SD Core register map (required for msm-v4 and below) + - SD Core register map (required for controllers earlier than msm-v5) + - CQE register map (Optional, CQE support is present on SDHC instance meant + for eMMC and version v4.2 and above) +- reg-names: When CQE register map is supplied, below reg-names are required + - "hc" for Host controller register map + - "core" for SD core register map + - "cqhci" for CQE register map - interrupts: Should contain an interrupt-specifiers for the interrupts: - Host controller interrupt (required) - pinctrl-names: Should contain only one value - "default". --Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., is a member of Code Aurora Forum, a Linux Foundation Collaborative Project