| Message ID | 20241022141828.618-1-quic_sachgupt@quicinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mmc: sdhci-msm: Slot indexing for distinguishing multiple SDCC instances | expand |
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. > > 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 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. >> >
On 10/25/2024 6:03 PM, Adrian Hunter wrote: > 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. > Sorry for misleading sentence, the tool I use is lauterbach Trace32 and when using Lauterbach Trace32 tool, having a global variable makes it easier to load and inspect dumps. It will be easy to quickly locate and analyze the sdhci_msm_host structure, which speeds up the debugging process. >> >>>> >>>> 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 Mon, Oct 28, 2024 at 04:10:37PM +0530, Sachin Gupta wrote: > > > On 10/25/2024 6:03 PM, Adrian Hunter wrote: > > 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. > > > > Sorry for misleading sentence, the tool I use is lauterbach Trace32 and when > using Lauterbach Trace32 tool, having a global variable makes it easier to > load and inspect dumps. It will be easy to quickly locate and analyze the > sdhci_msm_host structure, which speeds up the debugging process. This still isn't a functional purpose. This is a debugging code for a particular developer debugging SDHCI issues. > > > > > 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 10/28/2024 5:56 PM, Dmitry Baryshkov wrote: > On Mon, Oct 28, 2024 at 04:10:37PM +0530, Sachin Gupta wrote: >> >> >> On 10/25/2024 6:03 PM, Adrian Hunter wrote: >>> 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. >>> >> >> Sorry for misleading sentence, the tool I use is lauterbach Trace32 and when >> using Lauterbach Trace32 tool, having a global variable makes it easier to >> load and inspect dumps. It will be easy to quickly locate and analyze the >> sdhci_msm_host structure, which speeds up the debugging process. > > This still isn't a functional purpose. This is a debugging code for a > particular developer debugging SDHCI issues. > Thanks for your comment and time we will explore more on this. >>>>>> 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.