[PATCH v10] mmc: Export host capabilities to debugfs.

[PATCH v10] mmc: Export host capabilities to debugfs.

[Harish Jenny K N]

This patch exports the host capabilities to debugfs

This idea of sharing host capabilities over debugfs
came up from Abbas Raza <Abbas_Raza@mentor.com>
Earlier discussions:
https://lkml.org/lkml/2018/3/5/357
https://www.spinics.net/lists/linux-mmc/msg48219.html

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Harish Jenny K N <harish_kandiga@mentor.com>
---

Changes in v10:
- minor review comment addressed. 
- Added "Reviewed-by" line

Changes in v9
- More code cleanup as suggested by Andy Shevchenko.

Changes in v8
- Changes to use for_each_set_bit as suggested by Andy Shevchenko.

Changes in v7
- Moved additional capabilities also to caps file as mentioned by Ulf Hansson
- compacting the code with macros

Changes in v6:
- Used DEFINE_SHOW_ATTRIBUTE

Changes in v5:
- Added parser logic in kernel by using debugfs_create_file  for caps and caps2 instead of debugfs_create_x32
- Changed Author

Changes in v4:
- Moved the creation of nodes to mmc_add_host_debugfs
- Exported caps2
- Renamed host_caps to caps

Changes in v3:
- Removed typecasting of &host->caps to (u32 *)

Changes in v2:
- Changed Author

 drivers/mmc/core/debugfs.c | 107 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 107 insertions(+)

diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
index c51e0c0..826d361 100644
--- a/drivers/mmc/core/debugfs.c
+++ b/drivers/mmc/core/debugfs.c
@@ -225,6 +225,110 @@ static int mmc_clock_opt_set(void *data, u64 val)
 DEFINE_SIMPLE_ATTRIBUTE(mmc_clock_fops, mmc_clock_opt_get, mmc_clock_opt_set,
 	"%llu\n");
 
+/*
+ * mmc_host_capabilities - MMC host capabilities
+ *
+ * This must be in sync with caps definitions in the mmc/host.h
+ */
+static const char * const mmc_host_capabilities[] = {
+	"4-bit transfers allowed",
+	"Supports MMC high-speed timing",
+	"Supports SD high-speed timing",
+	"Can signal pending SDIO IRQs",
+	"Talks only SPI protocols",
+	"Needs polling for card-detection",
+	"8 bit transfers allowed",
+	"Suspends (e)MMC/SD at idle",
+	"Nonremovable",
+	"Waits while card is busy",
+	"Allows erase/trim commands",
+	"Supports DDR mode at 3.3V",
+	"Supports DDR mode at 1.8V",
+	"Supports DDR mode at 1.2V",
+	"Can power off after boot",
+	"CMD14/CMD19 bus width ok",
+	"Supports UHS SDR12 mode",
+	"Supports UHS SDR25 mode",
+	"Supports UHS SDR50 mode",
+	"Supports UHS SDR104 mode",
+	"Supports UHS DDR50 mode",
+	"Unknown (bit 21)",
+	"Unknown (bit 22)",
+	"Supports Driver Type A",
+	"Supports Driver Type C",
+	"Supports Driver Type D",
+	"Unknown (bit 26)",
+	"RW reqs can be completed within mmc_request_done()",
+	"Supports Enable card detect wake",
+	"Can send commands during data transfer",
+	"CMD23 supported",
+	"Supports Hardware reset"
+};
+
+/*
+ * mmc_host_capabilities2 - MMC host additional capabilities
+ *
+ * This must be in sync with caps2 definitions in the mmc/host.h
+ */
+static const char * const mmc_host_capabilities2[] = {
+	"No access to Boot partition",
+	"Unknown (bit 1)",
+	"Can do full power cycle",
+	"Unknown (bit 3)",
+	"Unknown (bit 4)",
+	"Supports HS200 1.8V SDR",
+	"Supports HS200 1.2V SDR",
+	"Unknown (bit 7)",
+	"Unknown (bit 8)",
+	"Unknown (bit 9)",
+	"Card-detect signal active high",
+	"Write-protect signal active high",
+	"Unknown (bit 12)",
+	"Unknown (bit 13)",
+	"Can do complete power cycle of the card",
+	"Supports HS400 1.8V",
+	"Support HS400 1.2V",
+	"SDIO IRQ - Nothread",
+	"No physical write protect pin, assume always read-write",
+	"Do not send SDIO commands during initialization",
+	"Supports enhanced strobe",
+	"Do not send SD commands during initialization",
+	"Do not send (e)MMC commands during initialization",
+	"Has eMMC command queue engine",
+	"CQE can issue a direct command",
+	"Unknown (bit 25)",
+	"Unknown (bit 26)",
+	"Unknown (bit 27)",
+	"Unknown (bit 28)",
+	"Unknown (bit 29)",
+	"Unknown (bit 30)",
+	"Unknown (bit 31)"
+};
+
+static int mmc_caps_show(struct seq_file *s, void *unused)
+{
+	struct mmc_host *host = s->private;
+	unsigned long caps = host->caps;
+	unsigned long caps2 = host->caps2;
+	int bit;
+
+	seq_puts(s, "MMC Host capabilities are:\n");
+	seq_puts(s, "=============================================\n");
+
+	for_each_set_bit(bit, &caps, ARRAY_SIZE(mmc_host_capabilities))
+		seq_printf(s, "%s\n", mmc_host_capabilities[bit]);
+
+	seq_puts(s, "=============================================\n");
+	seq_puts(s, "MMC Host additional capabilities are:\n");
+	seq_puts(s, "=============================================\n");
+
+	for_each_set_bit(bit, &caps2, ARRAY_SIZE(mmc_host_capabilities2))
+		seq_printf(s, "%s\n", mmc_host_capabilities2[bit]);
+
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(mmc_caps);
+
 void mmc_add_host_debugfs(struct mmc_host *host)
 {
 	struct dentry *root;
@@ -243,6 +347,9 @@ void mmc_add_host_debugfs(struct mmc_host *host)
 	if (!debugfs_create_file("ios", S_IRUSR, root, host, &mmc_ios_fops))
 		goto err_node;
 
+	if (!debugfs_create_file("caps", 0400, root, host, &mmc_caps_fops))
+		goto err_node;
+
 	if (!debugfs_create_file("clock", S_IRUSR | S_IWUSR, root, host,
 			&mmc_clock_fops))
 		goto err_node;
-- 
1.9.1

Re: [PATCH v10] mmc: Export host capabilities to debugfs.

[Ulf Hansson]

On 13 March 2018 at 06:10, Harish Jenny K N <harish_kandiga@mentor.com> wrote:
> This patch exports the host capabilities to debugfs
>
> This idea of sharing host capabilities over debugfs
> came up from Abbas Raza <Abbas_Raza@mentor.com>
> Earlier discussions:
> https://lkml.org/lkml/2018/3/5/357
> https://www.spinics.net/lists/linux-mmc/msg48219.html
>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Harish Jenny K N <harish_kandiga@mentor.com>
> ---
>
> Changes in v10:
> - minor review comment addressed.
> - Added "Reviewed-by" line
>
> Changes in v9
> - More code cleanup as suggested by Andy Shevchenko.
>
> Changes in v8
> - Changes to use for_each_set_bit as suggested by Andy Shevchenko.
>
> Changes in v7
> - Moved additional capabilities also to caps file as mentioned by Ulf Hansson
> - compacting the code with macros
>
> Changes in v6:
> - Used DEFINE_SHOW_ATTRIBUTE
>
> Changes in v5:
> - Added parser logic in kernel by using debugfs_create_file  for caps and caps2 instead of debugfs_create_x32
> - Changed Author
>
> Changes in v4:
> - Moved the creation of nodes to mmc_add_host_debugfs
> - Exported caps2
> - Renamed host_caps to caps
>
> Changes in v3:
> - Removed typecasting of &host->caps to (u32 *)
>
> Changes in v2:
> - Changed Author
>
>  drivers/mmc/core/debugfs.c | 107 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 107 insertions(+)
>
> diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
> index c51e0c0..826d361 100644
> --- a/drivers/mmc/core/debugfs.c
> +++ b/drivers/mmc/core/debugfs.c
> @@ -225,6 +225,110 @@ static int mmc_clock_opt_set(void *data, u64 val)
>  DEFINE_SIMPLE_ATTRIBUTE(mmc_clock_fops, mmc_clock_opt_get, mmc_clock_opt_set,
>         "%llu\n");
>
> +/*
> + * mmc_host_capabilities - MMC host capabilities
> + *
> + * This must be in sync with caps definitions in the mmc/host.h
> + */
> +static const char * const mmc_host_capabilities[] = {
> +       "4-bit transfers allowed",
> +       "Supports MMC high-speed timing",
> +       "Supports SD high-speed timing",
> +       "Can signal pending SDIO IRQs",
> +       "Talks only SPI protocols",
> +       "Needs polling for card-detection",
> +       "8 bit transfers allowed",
> +       "Suspends (e)MMC/SD at idle",
> +       "Nonremovable",
> +       "Waits while card is busy",
> +       "Allows erase/trim commands",
> +       "Supports DDR mode at 3.3V",
> +       "Supports DDR mode at 1.8V",
> +       "Supports DDR mode at 1.2V",
> +       "Can power off after boot",
> +       "CMD14/CMD19 bus width ok",
> +       "Supports UHS SDR12 mode",
> +       "Supports UHS SDR25 mode",
> +       "Supports UHS SDR50 mode",
> +       "Supports UHS SDR104 mode",
> +       "Supports UHS DDR50 mode",
> +       "Unknown (bit 21)",
> +       "Unknown (bit 22)",
> +       "Supports Driver Type A",
> +       "Supports Driver Type C",
> +       "Supports Driver Type D",
> +       "Unknown (bit 26)",
> +       "RW reqs can be completed within mmc_request_done()",
> +       "Supports Enable card detect wake",
> +       "Can send commands during data transfer",
> +       "CMD23 supported",
> +       "Supports Hardware reset"
> +};
> +
> +/*
> + * mmc_host_capabilities2 - MMC host additional capabilities
> + *
> + * This must be in sync with caps2 definitions in the mmc/host.h
> + */
> +static const char * const mmc_host_capabilities2[] = {
> +       "No access to Boot partition",
> +       "Unknown (bit 1)",
> +       "Can do full power cycle",
> +       "Unknown (bit 3)",
> +       "Unknown (bit 4)",
> +       "Supports HS200 1.8V SDR",
> +       "Supports HS200 1.2V SDR",
> +       "Unknown (bit 7)",
> +       "Unknown (bit 8)",
> +       "Unknown (bit 9)",
> +       "Card-detect signal active high",
> +       "Write-protect signal active high",
> +       "Unknown (bit 12)",
> +       "Unknown (bit 13)",
> +       "Can do complete power cycle of the card",
> +       "Supports HS400 1.8V",
> +       "Support HS400 1.2V",
> +       "SDIO IRQ - Nothread",
> +       "No physical write protect pin, assume always read-write",
> +       "Do not send SDIO commands during initialization",
> +       "Supports enhanced strobe",
> +       "Do not send SD commands during initialization",
> +       "Do not send (e)MMC commands during initialization",
> +       "Has eMMC command queue engine",
> +       "CQE can issue a direct command",
> +       "Unknown (bit 25)",
> +       "Unknown (bit 26)",
> +       "Unknown (bit 27)",
> +       "Unknown (bit 28)",
> +       "Unknown (bit 29)",
> +       "Unknown (bit 30)",
> +       "Unknown (bit 31)"
> +};

Honestly, I don't like this, but maybe other people do, then I am fine
with this approach.

If were to decide, I would just rather print the caps field in a
hexadecimal bit form and leave the translation to the user.

> +
> +static int mmc_caps_show(struct seq_file *s, void *unused)
> +{
> +       struct mmc_host *host = s->private;
> +       unsigned long caps = host->caps;
> +       unsigned long caps2 = host->caps2;
> +       int bit;
> +
> +       seq_puts(s, "MMC Host capabilities are:\n");
> +       seq_puts(s, "=============================================\n");
> +
> +       for_each_set_bit(bit, &caps, ARRAY_SIZE(mmc_host_capabilities))
> +               seq_printf(s, "%s\n", mmc_host_capabilities[bit]);
> +
> +       seq_puts(s, "=============================================\n");
> +       seq_puts(s, "MMC Host additional capabilities are:\n");
> +       seq_puts(s, "=============================================\n");
> +
> +       for_each_set_bit(bit, &caps2, ARRAY_SIZE(mmc_host_capabilities2))
> +               seq_printf(s, "%s\n", mmc_host_capabilities2[bit]);
> +
> +       return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(mmc_caps);
> +
>  void mmc_add_host_debugfs(struct mmc_host *host)
>  {
>         struct dentry *root;
> @@ -243,6 +347,9 @@ void mmc_add_host_debugfs(struct mmc_host *host)
>         if (!debugfs_create_file("ios", S_IRUSR, root, host, &mmc_ios_fops))
>                 goto err_node;
>
> +       if (!debugfs_create_file("caps", 0400, root, host, &mmc_caps_fops))
> +               goto err_node;
> +
>         if (!debugfs_create_file("clock", S_IRUSR | S_IWUSR, root, host,
>                         &mmc_clock_fops))
>                 goto err_node;
> --
> 1.9.1
>

Kind regards
Uffe

Re: [PATCH v10] mmc: Export host capabilities to debugfs.

[Andy Shevchenko]

On Thu, 2018-03-15 at 11:12 +0100, Ulf Hansson wrote:
> On 13 March 2018 at 06:10, Harish Jenny K N <harish_kandiga@mentor.com
> > wrote:
> > 

> Honestly, I don't like this, but maybe other people do, then I am fine
> with this approach.
> 
> If were to decide, I would just rather print the caps field in a
> hexadecimal bit form and leave the translation to the user.

A compromise would be to print both:

0xHHHHHHHH\n
Description of each enabled field, one per line


Another format would be:

Bit XX: Description of a field

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v10] mmc: Export host capabilities to debugfs.

[Ulf Hansson]

On 15 March 2018 at 11:26, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Thu, 2018-03-15 at 11:12 +0100, Ulf Hansson wrote:
>> On 13 March 2018 at 06:10, Harish Jenny K N <harish_kandiga@mentor.com
>> > wrote:
>> >
>
>> Honestly, I don't like this, but maybe other people do, then I am fine
>> with this approach.
>>
>> If were to decide, I would just rather print the caps field in a
>> hexadecimal bit form and leave the translation to the user.
>
> A compromise would be to print both:
>
> 0xHHHHHHHH\n
> Description of each enabled field, one per line
>
>
> Another format would be:
>
> Bit XX: Description of a field

If we were to print the description, there is no point in printing the
bits in hex. Or is it?

As I said, if you and other folkz thinks this is valuable, then I am
fine as well. Just saying, it's not my preferred option.

Kind regards
Uffe

Re: [PATCH v10] mmc: Export host capabilities to debugfs.

[Andy Shevchenko]

On Thu, 2018-03-15 at 13:29 +0100, Ulf Hansson wrote:
> On 15 March 2018 at 11:26, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Thu, 2018-03-15 at 11:12 +0100, Ulf Hansson wrote:
> > > On 13 March 2018 at 06:10, Harish Jenny K N <harish_kandiga@mentor
> > > .com
> > > > wrote:
> > > > 
> > > Honestly, I don't like this, but maybe other people do, then I am
> > > fine
> > > with this approach.
> > > 
> > > If were to decide, I would just rather print the caps field in a
> > > hexadecimal bit form and leave the translation to the user.
> > 
> > A compromise would be to print both:
> > 
> > 0xHHHHHHHH\n
> > Description of each enabled field, one per line
> > 
> > 
> > Another format would be:
> > 
> > Bit XX: Description of a field
> 
> If we were to print the description, there is no point in printing the
> bits in hex. Or is it?

It is helpful for scripting (since it's debugfs, under scripting I mean
something rather for test suites than for actual use).

> As I said, if you and other folkz thinks this is valuable, then I am
> fine as well. Just saying, it's not my preferred option.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v10] mmc: Export host capabilities to debugfs.

[Harish Jenny K N]

On Thursday 15 March 2018 05:59 PM, Ulf Hansson wrote:
> On 15 March 2018 at 11:26, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
>> On Thu, 2018-03-15 at 11:12 +0100, Ulf Hansson wrote:
>>> On 13 March 2018 at 06:10, Harish Jenny K N <harish_kandiga@mentor.com
>>>> wrote:
>>>>
>>> Honestly, I don't like this, but maybe other people do, then I am fine
>>> with this approach.
>>>
>>> If were to decide, I would just rather print the caps field in a
>>> hexadecimal bit form and leave the translation to the user.
>> A compromise would be to print both:
>>
>> 0xHHHHHHHH\n
>> Description of each enabled field, one per line
>>
>>
>> Another format would be:
>>
>> Bit XX: Description of a field
> If we were to print the description, there is no point in printing the
> bits in hex. Or is it?
Yes. I also do not see the use of printing hex value if we are printing the description.

>
> As I said, if you and other folkz thinks this is valuable, then I am
> fine as well. Just saying, it's not my preferred option.
>
>


I just want to inform that the idea of printing the description came after discussion in https://www.spinics.net/lists/linux-mmc/msg48246.html, where it was decided adding utility in mmc-utils was not going to work ( reason:  We may very well be changing the bit offsets for the caps and caps2 in the mmc kernel header, keeping a copy of them is not a good idea. It's just a matter of *when* it will break).

On Thu, 2018-03-15 at 11:12 +0100, Ulf Hansson wrote:
> If were to decide, I would just rather print the caps field in a hexadecimal bit form and leave the translation to the user.

I think translation becomes difficult for the above reason and hence I would prefer printing the description.

Note: Printing values in Hex was the original idea and it is also available in https://www.spinics.net/lists/linux-mmc/msg48213.html just in case if it is required.




Thanks & Regards,
Harish Jenny K N

Re: [PATCH v10] mmc: Export host capabilities to debugfs.

[Ulf Hansson]

On 16 March 2018 at 05:20, Harish Jenny K N <harish_kandiga@mentor.com> wrote:
>
>
> On Thursday 15 March 2018 05:59 PM, Ulf Hansson wrote:
>> On 15 March 2018 at 11:26, Andy Shevchenko
>> <andriy.shevchenko@linux.intel.com> wrote:
>>> On Thu, 2018-03-15 at 11:12 +0100, Ulf Hansson wrote:
>>>> On 13 March 2018 at 06:10, Harish Jenny K N <harish_kandiga@mentor.com
>>>>> wrote:
>>>>>
>>>> Honestly, I don't like this, but maybe other people do, then I am fine
>>>> with this approach.
>>>>
>>>> If were to decide, I would just rather print the caps field in a
>>>> hexadecimal bit form and leave the translation to the user.
>>> A compromise would be to print both:
>>>
>>> 0xHHHHHHHH\n
>>> Description of each enabled field, one per line
>>>
>>>
>>> Another format would be:
>>>
>>> Bit XX: Description of a field
>> If we were to print the description, there is no point in printing the
>> bits in hex. Or is it?
> Yes. I also do not see the use of printing hex value if we are printing the description.
>
>>
>> As I said, if you and other folkz thinks this is valuable, then I am
>> fine as well. Just saying, it's not my preferred option.
>>
>>
>
>
> I just want to inform that the idea of printing the description came after discussion in https://www.spinics.net/lists/linux-mmc/msg48246.html, where it was decided adding utility in mmc-utils was not going to work ( reason:  We may very well be changing the bit offsets for the caps and caps2 in the mmc kernel header, keeping a copy of them is not a good idea. It's just a matter of *when* it will break).

I recall. However, I didn't realize all these strings were going to be
needed. :-)

>
> On Thu, 2018-03-15 at 11:12 +0100, Ulf Hansson wrote:
>> If were to decide, I would just rather print the caps field in a hexadecimal bit form and leave the translation to the user.
>
> I think translation becomes difficult for the above reason and hence I would prefer printing the description.

Okay.

>
> Note: Printing values in Hex was the original idea and it is also available in https://www.spinics.net/lists/linux-mmc/msg48213.html just in case if it is required.

Yeah, so maybe I should apply that one, then we can take it from there!?

Kind regards
Uffe

Re: [PATCH v10] mmc: Export host capabilities to debugfs.

[Harish Jenny K N]

On Friday 16 March 2018 01:27 PM, Ulf Hansson wrote:
> On 16 March 2018 at 05:20, Harish Jenny K N <harish_kandiga@mentor.com> wrote:
>>
>> On Thursday 15 March 2018 05:59 PM, Ulf Hansson wrote:
>>> On 15 March 2018 at 11:26, Andy Shevchenko
>>> <andriy.shevchenko@linux.intel.com> wrote:
>>>> On Thu, 2018-03-15 at 11:12 +0100, Ulf Hansson wrote:
>>>>> On 13 March 2018 at 06:10, Harish Jenny K N <harish_kandiga@mentor.com
>>>>>> wrote:
>>>>>>
>>>>> Honestly, I don't like this, but maybe other people do, then I am fine
>>>>> with this approach.
>>>>>
>>>>> If were to decide, I would just rather print the caps field in a
>>>>> hexadecimal bit form and leave the translation to the user.
>>>> A compromise would be to print both:
>>>>
>>>> 0xHHHHHHHH\n
>>>> Description of each enabled field, one per line
>>>>
>>>>
>>>> Another format would be:
>>>>
>>>> Bit XX: Description of a field
>>> If we were to print the description, there is no point in printing the
>>> bits in hex. Or is it?
>> Yes. I also do not see the use of printing hex value if we are printing the description.
>>
>>> As I said, if you and other folkz thinks this is valuable, then I am
>>> fine as well. Just saying, it's not my preferred option.
>>>
>>>
>>
>> I just want to inform that the idea of printing the description came after discussion in https://www.spinics.net/lists/linux-mmc/msg48246.html, where it was decided adding utility in mmc-utils was not going to work ( reason:  We may very well be changing the bit offsets for the caps and caps2 in the mmc kernel header, keeping a copy of them is not a good idea. It's just a matter of *when* it will break).
> I recall. However, I didn't realize all these strings were going to be
> needed. :-)
>
>> On Thu, 2018-03-15 at 11:12 +0100, Ulf Hansson wrote:
>>> If were to decide, I would just rather print the caps field in a hexadecimal bit form and leave the translation to the user.
>> I think translation becomes difficult for the above reason and hence I would prefer printing the description.
> Okay.
>
>> Note: Printing values in Hex was the original idea and it is also available in https://www.spinics.net/lists/linux-mmc/msg48213.html just in case if it is required.
> Yeah, so maybe I should apply that one, then we can take it from there!?



Fine with both the approach. Please consider this.


Thanks,
Harish Jenny K N