Re: [PATCH v10] mmc: Export host capabilities to debugfs.
On Friday 16 March 2018 01:27 PM, Ulf Hansson wrote: > On 16 March 2018 at 05:20, Harish Jenny K N wrote: >> >> On Thursday 15 March 2018 05:59 PM, Ulf Hansson wrote: >>> On 15 March 2018 at 11:26, Andy Shevchenko >>> wrote: On Thu, 2018-03-15 at 11:12 +0100, Ulf Hansson wrote: > On 13 March 2018 at 06:10, Harish Jenny K N > 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: 0x\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
Re: [PATCH v10] mmc: Export host capabilities to debugfs.
On 16 March 2018 at 05:20, Harish Jenny K N wrote: > > > On Thursday 15 March 2018 05:59 PM, Ulf Hansson wrote: >> On 15 March 2018 at 11:26, Andy Shevchenko >> wrote: >>> On Thu, 2018-03-15 at 11:12 +0100, Ulf Hansson wrote: On 13 March 2018 at 06:10, Harish Jenny K N 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: >>> >>> 0x\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.
On Thursday 15 March 2018 05:59 PM, Ulf Hansson wrote: > On 15 March 2018 at 11:26, Andy Shevchenko > wrote: >> On Thu, 2018-03-15 at 11:12 +0100, Ulf Hansson wrote: >>> On 13 March 2018 at 06:10, Harish Jenny K N >>> 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: >> >> 0x\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.
On Thu, 2018-03-15 at 13:29 +0100, Ulf Hansson wrote: > On 15 March 2018 at 11:26, Andy Shevchenko > wrote: > > On Thu, 2018-03-15 at 11:12 +0100, Ulf Hansson wrote: > > > On 13 March 2018 at 06:10, Harish Jenny K N > > .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: > > > > 0x\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 Intel Finland Oy
Re: [PATCH v10] mmc: Export host capabilities to debugfs.
On 15 March 2018 at 11:26, Andy Shevchenko wrote: > On Thu, 2018-03-15 at 11:12 +0100, Ulf Hansson wrote: >> On 13 March 2018 at 06:10, Harish Jenny K N > > 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: > > 0x\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.
On Thu, 2018-03-15 at 11:12 +0100, Ulf Hansson wrote: > On 13 March 2018 at 06:10, Harish Jenny K N > 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: 0x\n Description of each enabled field, one per line Another format would be: Bit XX: Description of a field -- Andy Shevchenko Intel Finland Oy
Re: [PATCH v10] mmc: Export host capabilities to debugfs.
On 13 March 2018 at 06:10, Harish Jenny K N wrote:
> This patch exports the host capabilities to debugfs
>
> This idea of sharing host capabilities over debugfs
> came up from Abbas Raza
> Earlier discussions:
> https://lkml.org/lkml/2018/3/5/357
> https://www.spinics.net/lists/linux-mmc/msg48219.html
>
> Reviewed-by: Andy Shevchenko
> Signed-off-by: Harish Jenny K N
> ---
>
> 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_
[PATCH v10] mmc: Export host capabilities to debugfs.
This patch exports the host capabilities to debugfs
This idea of sharing host capabilities over debugfs
came up from Abbas Raza
Earlier discussions:
https://lkml.org/lkml/2018/3/5/357
https://www.spinics.net/lists/linux-mmc/msg48219.html
Reviewed-by: Andy Shevchenko
Signed-off-by: Harish Jenny K N
---
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))
+
