| Message ID | 20241022141828.618-1-quic_sachgupt@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | mmc: sdhci-msm: Slot indexing for distinguishing multiple SDCC instances | expand |
On 10/24/2024 4:38 PM, Adrian Hunter wrote: > On 22/10/24 17:18, Sachin Gupta wrote: >> This update addresses the requirement for accurate slot indexing >> in the sdhci-msm driver to differentiate between multiple SDCC >> (Secure Digital Card Controller) instances, such as eMMC, SD card, >> and SDIO. >> >> Additionally, it revises the slot indexing logic to comply with >> the new device tree (DT) specifications. > > This patch seems incomplete because all it does is assign a global > variable which is never used again. > Qualcomm internal debugging tools utilize this global variable to access and differentiate between all the instance's sdhci_msm_host data structure (eMMC, SD card, and SDIO). >> >> Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com> >> Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com> >> Signed-off-by: Maramaina Naresh <quic_mnaresh@quicinc.com> >> Signed-off-by: Ziqi Chen <quic_ziqichen@quicinc.com> >> Signed-off-by: Sachin Gupta <quic_sachgupt@quicinc.com> >> --- >> drivers/mmc/host/sdhci-msm.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c >> index e113b99a3eab..3cb79117916f 100644 >> --- a/drivers/mmc/host/sdhci-msm.c >> +++ b/drivers/mmc/host/sdhci-msm.c >> @@ -292,6 +292,8 @@ struct sdhci_msm_host { >> bool vqmmc_enabled; >> }; >> >> +static struct sdhci_msm_host *sdhci_slot[3]; >> + >> static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host) >> { >> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); >> @@ -2426,6 +2428,14 @@ static int sdhci_msm_probe(struct platform_device *pdev) >> if (ret) >> goto pltfm_free; >> >> + if (node) { >> + ret = of_alias_get_id(pdev->dev.of_node, "mmc"); >> + if (ret < 0) >> + dev_err(&pdev->dev, "get slot index failed %d\n", ret); >> + else >> + sdhci_slot[ret] = msm_host; >> + } >> + >> /* >> * Based on the compatible string, load the required msm host info from >> * the data associated with the version info. >
On Fri, Oct 25, 2024 at 04:07:52PM +0530, Sachin Gupta wrote: > > > On 10/24/2024 4:38 PM, Adrian Hunter wrote: > > On 22/10/24 17:18, Sachin Gupta wrote: > > > This update addresses the requirement for accurate slot indexing > > > in the sdhci-msm driver to differentiate between multiple SDCC > > > (Secure Digital Card Controller) instances, such as eMMC, SD card, > > > and SDIO. > > > > > > Additionally, it revises the slot indexing logic to comply with > > > the new device tree (DT) specifications. > > > > This patch seems incomplete because all it does is assign a global > > variable which is never used again. > > > > Qualcomm internal debugging tools utilize this global variable to > access and differentiate between all the instance's sdhci_msm_host > data structure (eMMC, SD card, and SDIO). "Internal debugging tool". Thank you. NAK from my point of view. It's upstream kernel, so none of the behind-the-closed-doors tools exist from its point of view. If you'd consider making it public and open-source, then we can discuss a best way for it to work. > > > Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com> > > > Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com> > > > Signed-off-by: Maramaina Naresh <quic_mnaresh@quicinc.com> > > > Signed-off-by: Ziqi Chen <quic_ziqichen@quicinc.com> > > > Signed-off-by: Sachin Gupta <quic_sachgupt@quicinc.com> > > > --- > > > drivers/mmc/host/sdhci-msm.c | 10 ++++++++++ > > > 1 file changed, 10 insertions(+) > > > > > > diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c > > > index e113b99a3eab..3cb79117916f 100644 > > > --- a/drivers/mmc/host/sdhci-msm.c > > > +++ b/drivers/mmc/host/sdhci-msm.c > > > @@ -292,6 +292,8 @@ struct sdhci_msm_host { > > > bool vqmmc_enabled; > > > }; > > > +static struct sdhci_msm_host *sdhci_slot[3]; > > > + > > > static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host) > > > { > > > struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > > > @@ -2426,6 +2428,14 @@ static int sdhci_msm_probe(struct platform_device *pdev) > > > if (ret) > > > goto pltfm_free; > > > + if (node) { > > > + ret = of_alias_get_id(pdev->dev.of_node, "mmc"); > > > + if (ret < 0) > > > + dev_err(&pdev->dev, "get slot index failed %d\n", ret); > > > + else > > > + sdhci_slot[ret] = msm_host; > > > + } > > > + > > > /* > > > * Based on the compatible string, load the required msm host info from > > > * the data associated with the version info. > > >
On 25/10/24 13:37, Sachin Gupta wrote: > > > On 10/24/2024 4:38 PM, Adrian Hunter wrote: >> On 22/10/24 17:18, Sachin Gupta wrote: >>> This update addresses the requirement for accurate slot indexing >>> in the sdhci-msm driver to differentiate between multiple SDCC >>> (Secure Digital Card Controller) instances, such as eMMC, SD card, >>> and SDIO. >>> >>> Additionally, it revises the slot indexing logic to comply with >>> the new device tree (DT) specifications. >> >> This patch seems incomplete because all it does is assign a global >> variable which is never used again. >> > > Qualcomm internal debugging tools utilize this global variable to > access and differentiate between all the instance's sdhci_msm_host > data structure (eMMC, SD card, and SDIO). The kernel does not accept code that does not serve a functional purpose. You could look at using eBPF or KGDB to get the information, otherwise you might just have to carry that kind of patch in your internal tree. > >>> >>> Signed-off-by: Ram Prakash Gupta <quic_rampraka@quicinc.com> >>> Signed-off-by: Sarthak Garg <quic_sartgarg@quicinc.com> >>> Signed-off-by: Maramaina Naresh <quic_mnaresh@quicinc.com> >>> Signed-off-by: Ziqi Chen <quic_ziqichen@quicinc.com> >>> Signed-off-by: Sachin Gupta <quic_sachgupt@quicinc.com> >>> --- >>> drivers/mmc/host/sdhci-msm.c | 10 ++++++++++ >>> 1 file changed, 10 insertions(+) >>> >>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c >>> index e113b99a3eab..3cb79117916f 100644 >>> --- a/drivers/mmc/host/sdhci-msm.c >>> +++ b/drivers/mmc/host/sdhci-msm.c >>> @@ -292,6 +292,8 @@ struct sdhci_msm_host { >>> bool vqmmc_enabled; >>> }; >>> +static struct sdhci_msm_host *sdhci_slot[3]; >>> + >>> static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host) >>> { >>> struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); >>> @@ -2426,6 +2428,14 @@ static int sdhci_msm_probe(struct platform_device *pdev) >>> if (ret) >>> goto pltfm_free; >>> + if (node) { >>> + ret = of_alias_get_id(pdev->dev.of_node, "mmc"); >>> + if (ret < 0) >>> + dev_err(&pdev->dev, "get slot index failed %d\n", ret); >>> + else >>> + sdhci_slot[ret] = msm_host; >>> + } >>> + >>> /* >>> * Based on the compatible string, load the required msm host info from >>> * the data associated with the version info. >> >
diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c index e113b99a3eab..3cb79117916f 100644 --- a/drivers/mmc/host/sdhci-msm.c +++ b/drivers/mmc/host/sdhci-msm.c @@ -292,6 +292,8 @@ struct sdhci_msm_host { bool vqmmc_enabled; }; +static struct sdhci_msm_host *sdhci_slot[3]; + static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host) { struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); @@ -2426,6 +2428,14 @@ static int sdhci_msm_probe(struct platform_device *pdev) if (ret) goto pltfm_free; + if (node) { + ret = of_alias_get_id(pdev->dev.of_node, "mmc"); + if (ret < 0) + dev_err(&pdev->dev, "get slot index failed %d\n", ret); + else + sdhci_slot[ret] = msm_host; + } + /* * Based on the compatible string, load the required msm host info from * the data associated with the version info.