Re: [PATCH v2] mmc-utils: Merge the lsmmc tool into mmc-utils

From: Baolin Wang <baolin.wang@linaro.org>
To: Chris Ball <chris@printf.net>
Cc: linux-mmc@vger.kernel.org, Ulf Hansson <ulf.hansson@linaro.org>,
	Mark Brown <broonie@kernel.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Sebastian Rasmussen <sebras@gmail.com>
Subject: Re: [PATCH v2] mmc-utils: Merge the lsmmc tool into mmc-utils
Date: Wed, 17 Feb 2016 10:31:13 +0800	[thread overview]
Message-ID: <CAMz4kuLnLeERAsM_piro_q_+V2P+OfjxNROKYNaSBr7qEUQ7bQ@mail.gmail.com> (raw)
In-Reply-To: <86fuwse854.fsf@void.printf.net>

On 17 February 2016 at 04:02, Chris Ball <chris@printf.net> wrote:
> Hi everyone,
>
> Sorry, I'm missing something hopefully-obvious about this patch:
>
> Which command does someone use to run the new ext_csd code in lsmmc?
>
> I thought that this patch would add lsmmc's ext_csd parsing to the
> existing "mmc extcsd read" command.  But it doesn't seem to do that --
> it duplicates the code, so there are two separate, unrelated extcsd
> parsers in:
>
>     * lsmmc.c:print_mmc_ext_csd()
>     * mmc_cmds.c:do_read_extcsd().
>
> I don't think that's what Ulf's suggestion intended -- as I read it,
> he was hoping to see the parsers merged to make one better parser.
> I'd like to see that too, though I understand that it's more work.
>
> And I'm still confused, as above, about how to actually use the new
> extcsd parser, because "mmc extcsd read" doesn't seem to be it, and
> the new commands added:
>
>     int do_read_scr(int argc, char **argv);
>     int do_read_cid(int argc, char **argv);
>     int do_read_csd(int argc, char **argv);
>
> .. don't seem to be it either, since they don't pass "EXT_CSD" to
> process_dir(), which is the only way I can see to activate lsmmc's
> ext_csd printer.  How's it supposed to work?

I want to merge the lsmmc code into mmc-utils firstly, then start to
modify the extcsd parser, but that seems not a good plan.

I'm also confused how to make one better extcsd parser. Remove the
do_read_extcsd() function in mmc_cmds.c file replacing with
print_mmc_ext_csd() function? Or merge the parser of
print_mmc_ext_csd() into do_read_extcsd(), then remove the
print_mmc_ext_csd() function? But I think it is not compatible to
merge different parser style together.

>
> Also:
>
> On Fri, Feb 05 2016, Baolin Wang wrote:
>> +void usage(void)
>> +int parse_opts(int argc, char **argv, struct config *config)
>
> These functions could be totally removed now, I think?

But there is one '- v' parameter, we just enable 'verbose' all the
time with removing the '-v' parameter?

>
> Thanks!
>
> - Chris.
> --
> Chris Ball   <http://printf.net/>

-- 
Baolin.wang
Best Regards