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.

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.
-- 
2.17.1

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 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.
> > > > 
> > > 
> > 
> 

-- 
With best wishes
Dmitry

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.
>>>>>
>>>>
>>>
>>
> 

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.
> > 
> 

-- 
With best wishes
Dmitry