More TMIO MMC variant. What is a preferred name?

More TMIO MMC variant. What is a preferred name?

[Masahiro Yamada]

Hi Wolfram, Simon, and Renesas engineers,


Currently, TMIO and Renesas SDHI share the same MMC core code.
I am planning to upstream one more variant for Socionext SoC family.

First, you may wonder why the tmio_mmc_core is used
for several SoC vendors.

As you may know, the provider of this IP is NOT Toshiba, but Panasonic.
So, the tmio_mmc_core is an unfortunate name.


Here is a short history of this IP.

Panasonic (the company name was Matsushita Electric Industrial at that time)
was one of the main board members of SD Association,
and developed an SD controller LSI.

This one  (MN5774)
https://industrial.panasonic.com/content/data/SC/ds/ds4/MN5774__E_discon.pdf?__hstc=231811840.07430159d50a3c91e72c280a7921bf0d.1506902400089.1506902400090.1506902400091.1&__hssc=231811840.1.1506902400092&__hsfp=1773666937

It was a dedicated controller chip connected to 16-bit bus.

That's why the register map of this IP is 16-bit oriented.
(Of course, the driver can support 32bit SoC bus with bus_shift == 1
or 64bit with bus_shift == 2.)

Besides, the IP was integrated into Panasonic own SoCs,
and also sold to several SoC vendors, including Toshiba, Renesas, and more.

I know Renesas extended this IP by themselves for 64bit bus, HS200,
internal DMAC etc,
but Panasonic still holds the right to sell the original of this IP.

Panasonic split the system LSI business out to Socionext Inc. in 2015.
That is why I want to upstream a new driver sharing the driver core code.
(Rather, the IP in Socionext SoCs is a lineage one)

The above is a history from the HW point of view
I heard from a hardware engineer who was involved in this IP
in Panasonic and Socionext.

Also, the log message of commit b6147490e6aac82fa2f4b9d7fce925d9891ebe8f
completely supports my comments.


>From the SW point of view, tmio_mmc was the first driver
merged in Linux.  Renesas largely expanded it for their SoC port.
Many thanks for big contributions.
In hindsight, the naming is unfortunate, though.


As far as I understood, TMIO is a name of MFD
(driver/mfd/tmio_core.c and its variants)


drivers/mmc/tmio_mmc.c is OK.

drivers/mmc/tmio_mmc_core.c is odd.
TMIO-MMC is just one user of this IP.


If you see  dw_mmc family,  drivers are named
in the form of <IP>-<SoC>.c


I know this is a big churn, but I'd like
to propose renaming like follows in a long run:

tmio_mmc_core.c      ->  mnsd.c              Core code of this IP
tmio_mmc.c           ->  mnsd-tmio.c         For MMC integrated in TMIO MFD
renesas_sdhi_core.c  ->  mnsd-renesas-core.c For Renesas SoCs
(or, mnsd-sdhi-core.c or whatever. please choose any favorite name)
                         mnsd-uniphier.c     For Socionext UniPhier SoCS



I see more strangeness.  Those mmc drivers must include <linux/mfd/tmio.h>,
but TMIO is unrelated to Renesas, Socionext.
We need to fix the interface.


If I can get consensus, I am very happy to
contribute for better organizing this IP variants.


CCing Marek Vasut, a contractor working for Renesas.
He and I are also working on SD card driver in U-Boot.
I want to introduce correct and systematic naming scheme
for a long-run maintainability and applicable to other projects
since Linux has a big influence in OSS.


-- 
Best Regards
Masahiro Yamada

Re: More TMIO MMC variant. What is a preferred name?

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 2973 bytes --]

Yamada-san,

Thank you for the detailed write up. I hope we find a good solution,
too.

I've got a few questions first:

> I know this is a big churn, but I'd like
> to propose renaming like follows in a long run:
> 
> tmio_mmc_core.c      ->  mnsd.c              Core code of this IP
> tmio_mmc.c           ->  mnsd-tmio.c         For MMC integrated in TMIO MFD
> renesas_sdhi_core.c  ->  mnsd-renesas-core.c For Renesas SoCs
> (or, mnsd-sdhi-core.c or whatever. please choose any favorite name)
>                          mnsd-uniphier.c     For Socionext UniPhier SoCS

Do you want to just change the filenames or the function names as well?

> I see more strangeness.  Those mmc drivers must include <linux/mfd/tmio.h>,
> but TMIO is unrelated to Renesas, Socionext.
> We need to fix the interface.

What kind of fix do you have in mind? Changing filenames or change
everything which has TMIO or tmio in the name?

> If I can get consensus, I am very happy to
> contribute for better organizing this IP variants.
> 
> CCing Marek Vasut, a contractor working for Renesas.
> He and I are also working on SD card driver in U-Boot.
> I want to introduce correct and systematic naming scheme
> for a long-run maintainability and applicable to other projects
> since Linux has a big influence in OSS.

Can you explain a bit more why the long-run maintainability gets improved?
I don't really see that yet and will try to explain below.

Disclaimer: I am contracted by Renesas but the views I am going to share
come from my personal point of view as the maintainer of this driver.

Preface: For me, names are just names. We have a few examples where the
"historical" names are kept because they just happened to be first. The
AT24 driver supports way more than only Atmel chips, and all the STMPE
have kept their names, although the SoC is called MXS these days. Which
also proves that names are just names, IP cores and companies are easily
renamed.

If we can rename things easily to match reality, I am not against it. In
this case, I wonder, though, if the rename is really easy? If you just
change the filenames, then we got a strange mix, because the function
names are still using tmio_*. If you change those function names, too,
then the resulting change is very intrusive. It could introduce errors,
looking up git history becomes a tad more tedious, and users may get
confused what is what. Seeing all this, I don't think such a change
would be helpful for the long-run maintainability.

I'd be much happier with a paragraph at the beginning of the "TMIO" core
file (or maybe all "TMIO" files, I don't care much) explaining the
situation like you did with your mail. But keeping the code largely
as-is.

I am open for discussion, though.

Again, this is my personal view. We will discuss things internally, too,
to see if there is also an "official" statement from Renesas.

Thanks,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: More TMIO MMC variant. What is a preferred name?

[Masahiro Yamada]

Hi Wolfram,

Thanks for your comments.


2017-11-09 2:53 GMT+09:00 Wolfram Sang <wsa@the-dreams.de>:
> Yamada-san,
>
> Thank you for the detailed write up. I hope we find a good solution,
> too.
>
> I've got a few questions first:
>
>> I know this is a big churn, but I'd like
>> to propose renaming like follows in a long run:
>>
>> tmio_mmc_core.c      ->  mnsd.c              Core code of this IP
>> tmio_mmc.c           ->  mnsd-tmio.c         For MMC integrated in TMIO MFD
>> renesas_sdhi_core.c  ->  mnsd-renesas-core.c For Renesas SoCs
>> (or, mnsd-sdhi-core.c or whatever. please choose any favorite name)
>>                          mnsd-uniphier.c     For Socionext UniPhier SoCS
>
> Do you want to just change the filenames or the function names as well?

If we decide to rename files,
functions and macros should be also renamed for consistency.

(I know this is a very invasive...)


>> I see more strangeness.  Those mmc drivers must include <linux/mfd/tmio.h>,
>> but TMIO is unrelated to Renesas, Socionext.
>> We need to fix the interface.
>
> What kind of fix do you have in mind? Changing filenames or change
> everything which has TMIO or tmio in the name?

Sorry, I missed lots of legacy boards under arch/sh/boards/.

At first, I imagined <linux/mfd/tmio.h> was used
for passing parameters from MFD to tmio-mmc.
Then, I thought it would be possible to copy parameters
to tmio_mmc_host in drivers/mmc/host/tmio_mmc.c
But, actually renesas also depends on platform-data,
so it does not seem so easy as I had imagined first.


We could split tmio_mmc_data out to include/linux/platform_data/tmio-mmc.h
but ideal goal might be split DT drivers and legacy board drivers.
(just an idea, not looking into the detail.)


>> If I can get consensus, I am very happy to
>> contribute for better organizing this IP variants.
>>
>> CCing Marek Vasut, a contractor working for Renesas.
>> He and I are also working on SD card driver in U-Boot.
>> I want to introduce correct and systematic naming scheme
>> for a long-run maintainability and applicable to other projects
>> since Linux has a big influence in OSS.
>
> Can you explain a bit more why the long-run maintainability gets improved?
> I don't really see that yet and will try to explain below.
>
> Disclaimer: I am contracted by Renesas but the views I am going to share
> come from my personal point of view as the maintainer of this driver.
>
> Preface: For me, names are just names. We have a few examples where the
> "historical" names are kept because they just happened to be first. The
> AT24 driver supports way more than only Atmel chips, and all the STMPE
> have kept their names, although the SoC is called MXS these days. Which
> also proves that names are just names, IP cores and companies are easily
> renamed.
>
> If we can rename things easily to match reality, I am not against it. In
> this case, I wonder, though, if the rename is really easy? If you just
> change the filenames, then we got a strange mix, because the function
> names are still using tmio_*. If you change those function names, too,
> then the resulting change is very intrusive.

Right.  This is a discussion point.

I admit it it a huge churn, but (I hope) one-time churn.


All functions / macros are prefixed with tmio_,
so sed scripts will do a good job.
It is just renaming, so no functional change will be added.
If we find a build error (probably reported by 0-day bot),
it will be easy to fix.


> It could introduce errors,
> looking up git history becomes a tad more tedious, and users may get
> confused what is what. Seeing all this, I don't think such a change
> would be helpful for the long-run maintainability.


Probably this could be the last chance for matching the code to reality
if we had (or, maybe it is too late...).

So, before adding a new driver for this IP,
this is worth discussing.

For DW and SDHCI, the naming scheme is really clean.
Kconfig options are prefixed with CONFIG_MMC_DW / CONFIG_MMC_SDHCI.
Same for file names and function names.

It is true that names are just names, but having a good name-space
is helpful to understand the code structure.



If my suggestion is NACK, do you have any suggestion for naming
new drivers?

If we make 'tmio_mmc_core' is really the right name,
do we want to do like  CONFIG_MMC_TMIO_UNIPHIER, tmio_mmc_uniphier.c?
(then rename renesas ones likewise, or maybe not?)

Or, is it completely "don't care" things, like Renesas is currently doing?


Linux is in a special position in open source world.
Various projects get influence from Linux.
U-Boot borrow code, ideas, and names from Linux
(barebox is more linux-addicted), so if we fix the history,
I thought this should be discussed in Linux community.


> I'd be much happier with a paragraph at the beginning of the "TMIO" core
> file (or maybe all "TMIO" files, I don't care much) explaining the
> situation like you did with your mail. But keeping the code largely
> as-is.
>
> I am open for discussion, though.
>
> Again, this is my personal view. We will discuss things internally, too,
> to see if there is also an "official" statement from Renesas.

OK.  Thanks.


> Thanks,
>
>    Wolfram
>



-- 
Best Regards
Masahiro Yamada

Re: More TMIO MMC variant. What is a preferred name?

[Wolfram Sang]

Hi Yamada-san,

> Sorry, I missed lots of legacy boards under arch/sh/boards/.
> 
> At first, I imagined <linux/mfd/tmio.h> was used
> for passing parameters from MFD to tmio-mmc.
> Then, I thought it would be possible to copy parameters
> to tmio_mmc_host in drivers/mmc/host/tmio_mmc.c
> But, actually renesas also depends on platform-data,
> so it does not seem so easy as I had imagined first.
> 
> 
> We could split tmio_mmc_data out to include/linux/platform_data/tmio-mmc.h
> but ideal goal might be split DT drivers and legacy board drivers.
> (just an idea, not looking into the detail.)

I think we need to sort out the high level view first. Do we want this
rename at all?

> > If we can rename things easily to match reality, I am not against it. In
> > this case, I wonder, though, if the rename is really easy? If you just
> > change the filenames, then we got a strange mix, because the function
> > names are still using tmio_*. If you change those function names, too,
> > then the resulting change is very intrusive.
> 
> Right.  This is a discussion point.
> 
> I admit it it a huge churn, but (I hope) one-time churn.

Churns have drawbacks independent of applying them once or multiple
times. git history becomes messy and customers having local patches will
have extra pain when updating the kernel. If it is worth it, OK.
However, I still don't see a real advantage yet.

> All functions / macros are prefixed with tmio_,
> so sed scripts will do a good job.

I have done that and it can be done, no doubt. But usually, this also
means fixing some leftovers manually and this can introduce the errors.
Like I said, can be done when needed but I am still wondering if it is
needed.

> It is just renaming, so no functional change will be added.
> If we find a build error (probably reported by 0-day bot),
> it will be easy to fix.

Sure. But there is also the case of a wrong auto-completion when
manually fixing things in a rare code path. Might not be needed here,
but could be. I wouldn't say the risk is 0%.

> Probably this could be the last chance for matching the code to reality
> if we had (or, maybe it is too late...).

Frankly, I'd think this is too late.

a) the documentation for TMIO/SDHI is hardly available even today.
   So, not much of a surprise if people hacking drivers get some
   details wrong. This is simply what happens when not working with
   the community timely enough. Absolutely no offence here, it is
   just the way it is.

b) but even today, you hardly know from datasheets if an IP core has
   been bought in. This is why I always compare register sets when
   getting new drivers for the I2C subsystem. Even the developer in
   that company usually doesn't know that there is a duplicate driver
   already.

So, this is something we have to live with anyhow.

> For DW and SDHCI, the naming scheme is really clean.
> Kconfig options are prefixed with CONFIG_MMC_DW / CONFIG_MMC_SDHCI.
> Same for file names and function names.

At least SDHCI had public documentation early, that helped. Dunno about
DesignWare. But I think for both it was clear from the beginning that
those are IP cores to be included by other vendors.

> It is true that names are just names, but having a good name-space
> is helpful to understand the code structure.

I agree. Though, good name space here means consistent in my book. I
don't think the code will be more readable if you exchange "tmio_" with
"mnsd_". It is helpful to have a different namespace for SDHI, but we
have that already.

> If my suggestion is NACK, do you have any suggestion for naming
> new drivers?
> 
> If we make 'tmio_mmc_core' is really the right name,
> do we want to do like  CONFIG_MMC_TMIO_UNIPHIER, tmio_mmc_uniphier.c?
> (then rename renesas ones likewise, or maybe not?)
> 
> Or, is it completely "don't care" things, like Renesas is currently doing?

It is not exactly "don't care". This naming scheme was suggested by Arnd
Bergmann [1] and agreed by us :) I don't think it is strange if
CONFIG_MMC_UNIPHIER includes CONFIG_MMC_TMIO_CORE. I have such setups in
I2C world, too. It might not be ideal, but is it really that confusing?

[1] http://www.spinics.net/lists/linux-mmc/msg38004.html

> Linux is in a special position in open source world.
> Various projects get influence from Linux.
> U-Boot borrow code, ideas, and names from Linux
> (barebox is more linux-addicted), so if we fix the history,

So, what about the idea in my last mail of adding the paragraph to the
source explaining the situation? This is far more educating than
changing the first four letters in a function name (which nobody really
knows what they mean anyhow). So, other projects can then decide to do
it better. Or, they copy the paragraph which in my book is again more
educational.

What do you think?

And maybe Ulf has something to add, too?

Kind regards,

   Wolfram

Re: More TMIO MMC variant. What is a preferred name?

[Ulf Hansson]

On 20 November 2017 at 21:11, Wolfram Sang <wsa@the-dreams.de> wrote:
> Hi Yamada-san,
>
>> Sorry, I missed lots of legacy boards under arch/sh/boards/.
>>
>> At first, I imagined <linux/mfd/tmio.h> was used
>> for passing parameters from MFD to tmio-mmc.
>> Then, I thought it would be possible to copy parameters
>> to tmio_mmc_host in drivers/mmc/host/tmio_mmc.c
>> But, actually renesas also depends on platform-data,
>> so it does not seem so easy as I had imagined first.
>>
>>
>> We could split tmio_mmc_data out to include/linux/platform_data/tmio-mmc.h
>> but ideal goal might be split DT drivers and legacy board drivers.
>> (just an idea, not looking into the detail.)

Let me just comment on this, because I have several times been tempted
to clean this up myself. :-)

I agree that include/linux/mfd/tmio.h needs a round of clean up.
However, most things that I care of is those parts that don't belong
in a public header file and should probably fit better in a new mfd
local header file, such as drivers/mfd/tmio.h.

The other parts in there, which are really ugly to me are those
wrapper functions of read*() and write*(). Some isn't even being
used....

Regarding platform data (like the struct tmio_mmc_data), I don't think
there are any gain in moving it to include/linux/platform_data/*,
because it's used by mfd, machine and mmc. So then it might as well
stay where it is.

>
> I think we need to sort out the high level view first. Do we want this
> rename at all?
>
>> > If we can rename things easily to match reality, I am not against it. In
>> > this case, I wonder, though, if the rename is really easy? If you just
>> > change the filenames, then we got a strange mix, because the function
>> > names are still using tmio_*. If you change those function names, too,
>> > then the resulting change is very intrusive.
>>
>> Right.  This is a discussion point.
>>
>> I admit it it a huge churn, but (I hope) one-time churn.
>
> Churns have drawbacks independent of applying them once or multiple
> times. git history becomes messy and customers having local patches will
> have extra pain when updating the kernel. If it is worth it, OK.
> However, I still don't see a real advantage yet.
>
>> All functions / macros are prefixed with tmio_,
>> so sed scripts will do a good job.
>
> I have done that and it can be done, no doubt. But usually, this also
> means fixing some leftovers manually and this can introduce the errors.
> Like I said, can be done when needed but I am still wondering if it is
> needed.
>
>> It is just renaming, so no functional change will be added.
>> If we find a build error (probably reported by 0-day bot),
>> it will be easy to fix.
>
> Sure. But there is also the case of a wrong auto-completion when
> manually fixing things in a rare code path. Might not be needed here,
> but could be. I wouldn't say the risk is 0%.
>
>> Probably this could be the last chance for matching the code to reality
>> if we had (or, maybe it is too late...).
>
> Frankly, I'd think this is too late.
>
> a) the documentation for TMIO/SDHI is hardly available even today.
>    So, not much of a surprise if people hacking drivers get some
>    details wrong. This is simply what happens when not working with
>    the community timely enough. Absolutely no offence here, it is
>    just the way it is.
>
> b) but even today, you hardly know from datasheets if an IP core has
>    been bought in. This is why I always compare register sets when
>    getting new drivers for the I2C subsystem. Even the developer in
>    that company usually doesn't know that there is a duplicate driver
>    already.
>
> So, this is something we have to live with anyhow.
>
>> For DW and SDHCI, the naming scheme is really clean.
>> Kconfig options are prefixed with CONFIG_MMC_DW / CONFIG_MMC_SDHCI.
>> Same for file names and function names.
>
> At least SDHCI had public documentation early, that helped. Dunno about
> DesignWare. But I think for both it was clear from the beginning that
> those are IP cores to be included by other vendors.
>
>> It is true that names are just names, but having a good name-space
>> is helpful to understand the code structure.
>
> I agree. Though, good name space here means consistent in my book. I
> don't think the code will be more readable if you exchange "tmio_" with
> "mnsd_". It is helpful to have a different namespace for SDHI, but we
> have that already.
>
>> If my suggestion is NACK, do you have any suggestion for naming
>> new drivers?
>>
>> If we make 'tmio_mmc_core' is really the right name,
>> do we want to do like  CONFIG_MMC_TMIO_UNIPHIER, tmio_mmc_uniphier.c?
>> (then rename renesas ones likewise, or maybe not?)
>>
>> Or, is it completely "don't care" things, like Renesas is currently doing?
>
> It is not exactly "don't care". This naming scheme was suggested by Arnd
> Bergmann [1] and agreed by us :) I don't think it is strange if
> CONFIG_MMC_UNIPHIER includes CONFIG_MMC_TMIO_CORE. I have such setups in
> I2C world, too. It might not be ideal, but is it really that confusing?
>
> [1] http://www.spinics.net/lists/linux-mmc/msg38004.html
>
>> Linux is in a special position in open source world.
>> Various projects get influence from Linux.
>> U-Boot borrow code, ideas, and names from Linux
>> (barebox is more linux-addicted), so if we fix the history,
>
> So, what about the idea in my last mail of adding the paragraph to the
> source explaining the situation? This is far more educating than
> changing the first four letters in a function name (which nobody really
> knows what they mean anyhow). So, other projects can then decide to do
> it better. Or, they copy the paragraph which in my book is again more
> educational.
>
> What do you think?
>
> And maybe Ulf has something to add, too?

I think my main concern is that the filenames should strive to
providing some information about the relationship between the
variants, which they aren't. Instead, one need look carefully in the
code to understand that, and because there are actually two level of
"core" layers for the Renesas case, this becomes confusing.

To address this, I actually think some renaming would be nice, but I
wouldn't go as far as renaming functions (because that mess things up
too much), but just files.

So what do you think of this?:

1) Rename tmio_core.c to tmio.c, and fold in some more information
about the history of the IP in the header of the file. Yeah, "tmio"
may not be the absolutely correct name, but on the other hand it
preserves consistency and I there are no need to rename any functions.
2) Rename tmio_mmc.h to tmio.h - and move potential tmio_mmc specific
bits to tmio_mmc.c.
3) Rename renesas_sdhi_core.c to tmio_renesas_sdhi.c. Again, I don't
think we need any functions to be renamed because of this change.
4) Rename renesas_sdhi.h to tmio_renesas_sdhi.h.

Following the naming strategy from the above, I the new file name for
new tmio variant should be tmio_uniphier.c.

Kind regards
Uffe

Re: More TMIO MMC variant. What is a preferred name?

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 978 bytes --]

Hi Ulf,

thanks for the feedback!

> So what do you think of this?:
> 
> 1) Rename tmio_core.c to tmio.c, and fold in some more information
> about the history of the IP in the header of the file. Yeah, "tmio"
> may not be the absolutely correct name, but on the other hand it
> preserves consistency and I there are no need to rename any functions.
> 2) Rename tmio_mmc.h to tmio.h - and move potential tmio_mmc specific
> bits to tmio_mmc.c.
> 3) Rename renesas_sdhi_core.c to tmio_renesas_sdhi.c. Again, I don't
> think we need any functions to be renamed because of this change.
> 4) Rename renesas_sdhi.h to tmio_renesas_sdhi.h.

I can agree to that. Clarifying 3), I think though, we should have:
	tmio_sdhi.c

and on top of that:

	tmio_sdhi_internal_dmac.c
	tmio_sdhi_sys_dmac.c

?

> Following the naming strategy from the above, I the new file name for
> new tmio variant should be tmio_uniphier.c.

Yes.

Kind regards,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: More TMIO MMC variant. What is a preferred name?

[Ulf Hansson]

On 21 November 2017 at 21:01, Wolfram Sang <wsa@the-dreams.de> wrote:
> Hi Ulf,
>
> thanks for the feedback!
>
>> So what do you think of this?:
>>
>> 1) Rename tmio_core.c to tmio.c, and fold in some more information
>> about the history of the IP in the header of the file. Yeah, "tmio"
>> may not be the absolutely correct name, but on the other hand it
>> preserves consistency and I there are no need to rename any functions.
>> 2) Rename tmio_mmc.h to tmio.h - and move potential tmio_mmc specific
>> bits to tmio_mmc.c.
>> 3) Rename renesas_sdhi_core.c to tmio_renesas_sdhi.c. Again, I don't
>> think we need any functions to be renamed because of this change.
>> 4) Rename renesas_sdhi.h to tmio_renesas_sdhi.h.
>
> I can agree to that. Clarifying 3), I think though, we should have:
>         tmio_sdhi.c
>
> and on top of that:
>
>         tmio_sdhi_internal_dmac.c
>         tmio_sdhi_sys_dmac.c
>
> ?

Even better!

>
>> Following the naming strategy from the above, I the new file name for
>> new tmio variant should be tmio_uniphier.c.
>
> Yes.
>

Great!

Br
Uffe

Re: More TMIO MMC variant. What is a preferred name?

[Masahiro Yamada]

2017-11-22 5:14 GMT+09:00 Ulf Hansson <ulf.hansson@linaro.org>:
> On 21 November 2017 at 21:01, Wolfram Sang <wsa@the-dreams.de> wrote:
>> Hi Ulf,
>>
>> thanks for the feedback!
>>
>>> So what do you think of this?:
>>>
>>> 1) Rename tmio_core.c to tmio.c, and fold in some more information
>>> about the history of the IP in the header of the file. Yeah, "tmio"
>>> may not be the absolutely correct name, but on the other hand it
>>> preserves consistency and I there are no need to rename any functions.
>>> 2) Rename tmio_mmc.h to tmio.h - and move potential tmio_mmc specific
>>> bits to tmio_mmc.c.
>>> 3) Rename renesas_sdhi_core.c to tmio_renesas_sdhi.c. Again, I don't
>>> think we need any functions to be renamed because of this change.
>>> 4) Rename renesas_sdhi.h to tmio_renesas_sdhi.h.
>>
>> I can agree to that. Clarifying 3), I think though, we should have:
>>         tmio_sdhi.c
>>
>> and on top of that:
>>
>>         tmio_sdhi_internal_dmac.c
>>         tmio_sdhi_sys_dmac.c
>>
>> ?
>
> Even better!
>
>>
>>> Following the naming strategy from the above, I the new file name for
>>> new tmio variant should be tmio_uniphier.c.
>>
>> Yes.
>>
>
> Great!
>
> Br
> Uffe
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


In my understanding, TMIO is a name of MFD
that includes some built-in peripherals:
drivers/usb/host/ohci-tmio.c
drivers/mtd/nand/tmio_nand.c



If you use "tmio" as a prefix of a group of MMC drivers,
it will look odd.

# core
obj-$(CONFIG_MMC_TMIO)          += tmio.o
# TMIO-builtin
obj-$(CONFIG_MMC_TMIO_MMC)      += tmio_mmc.o
# Renesas SoCs
obj-$(CONFIG_MMC_TMIO_SDHI)     += tmio_sdhi.o
# UniPhier SoCs
obj-$(CONFIG_MMC_TMIO_UNIPHIER) += tmio_uniphier.o

This is funny,
especially the relation between tmio.o and tmio_mmc.o

The "mmc" in the "tmio_mmc.o"
is not a platform name.

This is even confusing.






If we use "tmio_mmc" prefix (like "dw_mmc"),

# core
obj-$(CONFIG_MMC_TMIO)          += tmio_mmc.o
# TMIO-builtin MMC
obj-$(CONFIG_MMC_TMIO_TMIO)     += tmio_mmc_tmio.o
# Reneses
obj-$(CONFIG_MMC_TMIO_SDHI)     += tmio_mmc_sdhi.o
# UniPhier
obj-$(CONFIG_MMC_TMIO_UNIPHIER) += tmio_mmc_uniphier.o


tmio_mmc_tmio.o  is a crazy name, but the syntax is clear.

The first "tmio_mmc"  means the IP name
(since we decide this is the right name)

The second "tmio" is the platform name
the comes from the TMIO MFD.




Thought?




-- 
Best Regards
Masahiro Yamada

Re: More TMIO MMC variant. What is a preferred name?

[Masahiro Yamada]

2017-11-22 10:16 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
> 2017-11-22 5:14 GMT+09:00 Ulf Hansson <ulf.hansson@linaro.org>:
>> On 21 November 2017 at 21:01, Wolfram Sang <wsa@the-dreams.de> wrote:
>>> Hi Ulf,
>>>
>>> thanks for the feedback!
>>>
>>>> So what do you think of this?:
>>>>
>>>> 1) Rename tmio_core.c to tmio.c, and fold in some more information
>>>> about the history of the IP in the header of the file. Yeah, "tmio"
>>>> may not be the absolutely correct name, but on the other hand it
>>>> preserves consistency and I there are no need to rename any functions.
>>>> 2) Rename tmio_mmc.h to tmio.h - and move potential tmio_mmc specific
>>>> bits to tmio_mmc.c.
>>>> 3) Rename renesas_sdhi_core.c to tmio_renesas_sdhi.c. Again, I don't
>>>> think we need any functions to be renamed because of this change.
>>>> 4) Rename renesas_sdhi.h to tmio_renesas_sdhi.h.
>>>
>>> I can agree to that. Clarifying 3), I think though, we should have:
>>>         tmio_sdhi.c
>>>
>>> and on top of that:
>>>
>>>         tmio_sdhi_internal_dmac.c
>>>         tmio_sdhi_sys_dmac.c
>>>
>>> ?
>>
>> Even better!
>>
>>>
>>>> Following the naming strategy from the above, I the new file name for
>>>> new tmio variant should be tmio_uniphier.c.
>>>
>>> Yes.
>>>
>>
>> Great!
>>
>> Br
>> Uffe
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
> In my understanding, TMIO is a name of MFD
> that includes some built-in peripherals:
> drivers/usb/host/ohci-tmio.c
> drivers/mtd/nand/tmio_nand.c
>
>
>
> If you use "tmio" as a prefix of a group of MMC drivers,
> it will look odd.
>
> # core
> obj-$(CONFIG_MMC_TMIO)          += tmio.o
> # TMIO-builtin
> obj-$(CONFIG_MMC_TMIO_MMC)      += tmio_mmc.o
> # Renesas SoCs
> obj-$(CONFIG_MMC_TMIO_SDHI)     += tmio_sdhi.o
> # UniPhier SoCs
> obj-$(CONFIG_MMC_TMIO_UNIPHIER) += tmio_uniphier.o
>
> This is funny,
> especially the relation between tmio.o and tmio_mmc.o
>
> The "mmc" in the "tmio_mmc.o"
> is not a platform name.
>
> This is even confusing.
>
>
>
>
>
>
> If we use "tmio_mmc" prefix (like "dw_mmc"),
>
> # core
> obj-$(CONFIG_MMC_TMIO)          += tmio_mmc.o
> # TMIO-builtin MMC
> obj-$(CONFIG_MMC_TMIO_TMIO)     += tmio_mmc_tmio.o
> # Reneses
> obj-$(CONFIG_MMC_TMIO_SDHI)     += tmio_mmc_sdhi.o
> # UniPhier
> obj-$(CONFIG_MMC_TMIO_UNIPHIER) += tmio_mmc_uniphier.o
>
>
> tmio_mmc_tmio.o  is a crazy name, but the syntax is clear.
>
> The first "tmio_mmc"  means the IP name
> (since we decide this is the right name)
>
> The second "tmio" is the platform name
> the comes from the TMIO MFD.
>
>
>
>
> Thought?
>
>
>
>
> --
> Best Regards
> Masahiro Yamada








> obj-$(CONFIG_MMC_TMIO)          += tmio.o


The module name "tmio.ko" sounds confusing to me.


If a user runs "insmod tmio"
it sounds like a driver for TMIO MFD is loaded,
but it is actually the core code of MMC.








-- 
Best Regards
Masahiro Yamada

Re: More TMIO MMC variant. What is a preferred name?

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 591 bytes --]


> If we use "tmio_mmc" prefix (like "dw_mmc"),
> 
> # core
> obj-$(CONFIG_MMC_TMIO)          += tmio_mmc.o
> # TMIO-builtin MMC
> obj-$(CONFIG_MMC_TMIO_TMIO)     += tmio_mmc_tmio.o
> # Reneses
> obj-$(CONFIG_MMC_TMIO_SDHI)     += tmio_mmc_sdhi.o
> # UniPhier
> obj-$(CONFIG_MMC_TMIO_UNIPHIER) += tmio_mmc_uniphier.o
> 
> 
> tmio_mmc_tmio.o  is a crazy name, but the syntax is clear.
> 
> The first "tmio_mmc"  means the IP name
> (since we decide this is the right name)
> 
> The second "tmio" is the platform name
> the comes from the TMIO MFD.

Also fine with me.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: More TMIO MMC variant. What is a preferred name?

[Masahiro Yamada]

2017-11-22 16:59 GMT+09:00 Wolfram Sang <wsa@the-dreams.de>:
>
>> If we use "tmio_mmc" prefix (like "dw_mmc"),
>>
>> # core
>> obj-$(CONFIG_MMC_TMIO)          += tmio_mmc.o
>> # TMIO-builtin MMC
>> obj-$(CONFIG_MMC_TMIO_TMIO)     += tmio_mmc_tmio.o
>> # Reneses
>> obj-$(CONFIG_MMC_TMIO_SDHI)     += tmio_mmc_sdhi.o
>> # UniPhier
>> obj-$(CONFIG_MMC_TMIO_UNIPHIER) += tmio_mmc_uniphier.o
>>
>>
>> tmio_mmc_tmio.o  is a crazy name, but the syntax is clear.
>>
>> The first "tmio_mmc"  means the IP name
>> (since we decide this is the right name)
>>
>> The second "tmio" is the platform name
>> the comes from the TMIO MFD.
>
> Also fine with me.
>

Then, all functions for the MFD-buitin one
start with tmio_tmio_mmc_ ...

Ugh.  No. Not fine.
I was kidding, sorry.


tmio_mmc_tmio.o is really ugly according to my common sense.


I'd like the IP name be something different from tmio,
but it seems NACK.
I have no more idea to suggest...


So, just keep everything as is.


I will borrow the core code from tmio_mmc_core.c
but I will name my driver as I like.



-- 
Best Regards
Masahiro Yamada

Re: More TMIO MMC variant. What is a preferred name?

[Ulf Hansson]

On 22 November 2017 at 02:16, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> 2017-11-22 5:14 GMT+09:00 Ulf Hansson <ulf.hansson@linaro.org>:
>> On 21 November 2017 at 21:01, Wolfram Sang <wsa@the-dreams.de> wrote:
>>> Hi Ulf,
>>>
>>> thanks for the feedback!
>>>
>>>> So what do you think of this?:
>>>>
>>>> 1) Rename tmio_core.c to tmio.c, and fold in some more information
>>>> about the history of the IP in the header of the file. Yeah, "tmio"
>>>> may not be the absolutely correct name, but on the other hand it
>>>> preserves consistency and I there are no need to rename any functions.
>>>> 2) Rename tmio_mmc.h to tmio.h - and move potential tmio_mmc specific
>>>> bits to tmio_mmc.c.
>>>> 3) Rename renesas_sdhi_core.c to tmio_renesas_sdhi.c. Again, I don't
>>>> think we need any functions to be renamed because of this change.
>>>> 4) Rename renesas_sdhi.h to tmio_renesas_sdhi.h.
>>>
>>> I can agree to that. Clarifying 3), I think though, we should have:
>>>         tmio_sdhi.c
>>>
>>> and on top of that:
>>>
>>>         tmio_sdhi_internal_dmac.c
>>>         tmio_sdhi_sys_dmac.c
>>>
>>> ?
>>
>> Even better!
>>
>>>
>>>> Following the naming strategy from the above, I the new file name for
>>>> new tmio variant should be tmio_uniphier.c.
>>>
>>> Yes.
>>>
>>
>> Great!
>>
>> Br
>> Uffe
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
> In my understanding, TMIO is a name of MFD
> that includes some built-in peripherals:
> drivers/usb/host/ohci-tmio.c
> drivers/mtd/nand/tmio_nand.c
>
>
>
> If you use "tmio" as a prefix of a group of MMC drivers,
> it will look odd.
>
> # core
> obj-$(CONFIG_MMC_TMIO)          += tmio.o
> # TMIO-builtin
> obj-$(CONFIG_MMC_TMIO_MMC)      += tmio_mmc.o
> # Renesas SoCs
> obj-$(CONFIG_MMC_TMIO_SDHI)     += tmio_sdhi.o
> # UniPhier SoCs
> obj-$(CONFIG_MMC_TMIO_UNIPHIER) += tmio_uniphier.o
>
> This is funny,
> especially the relation between tmio.o and tmio_mmc.o
>
> The "mmc" in the "tmio_mmc.o"
> is not a platform name.
>
> This is even confusing.

Well, still I think it's quite much better than what we have today.

[...]

Kind regards
Uffe

Re: More TMIO MMC variant. What is a preferred name?

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 364 bytes --]


Another try:

> # core
> obj-$(CONFIG_MMC_TMIO_CORE)		+= tmio_mmc_core.o
> # TMIO-builtin MMC
> obj-$(CONFIG_MMC_TMIO)		+= tmio_mmc_original.o
> # Reneses
> obj-$(CONFIG_MMC_SDHI)		+= tmio_mmc_sdhi_core.o
> # UniPhier
> obj-$(CONFIG_MMC_UNIPHIER)		+= tmio_mmc_uniphier.o

I left the Kconfig symbols for easier updating and concentrated on the
filenames for now.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: More TMIO MMC variant. What is a preferred name?

[Ulf Hansson]

On 22 November 2017 at 18:08, Wolfram Sang <wsa@the-dreams.de> wrote:
>
> Another try:
>
>> # core
>> obj-$(CONFIG_MMC_TMIO_CORE)           += tmio_mmc_core.o
>> # TMIO-builtin MMC
>> obj-$(CONFIG_MMC_TMIO)                += tmio_mmc_original.o
>> # Reneses
>> obj-$(CONFIG_MMC_SDHI)                += tmio_mmc_sdhi_core.o
>> # UniPhier
>> obj-$(CONFIG_MMC_UNIPHIER)            += tmio_mmc_uniphier.o
>
> I left the Kconfig symbols for easier updating and concentrated on the
> filenames for now.
>

Looks good to me!

Kind regards
Uffe

Re: More TMIO MMC variant. What is a preferred name?

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 513 bytes --]


> > Another try:
> >
> >> # core
> >> obj-$(CONFIG_MMC_TMIO_CORE)           += tmio_mmc_core.o
> >> # TMIO-builtin MMC
> >> obj-$(CONFIG_MMC_TMIO)                += tmio_mmc_original.o
> >> # Reneses
> >> obj-$(CONFIG_MMC_SDHI)                += tmio_mmc_sdhi_core.o
> >> # UniPhier
> >> obj-$(CONFIG_MMC_UNIPHIER)            += tmio_mmc_uniphier.o
> >
> > I left the Kconfig symbols for easier updating and concentrated on the
> > filenames for now.
> >
> 
> Looks good to me!

Yamada-san?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: More TMIO MMC variant. What is a preferred name?

[Masahiro Yamada]

2017-11-28 2:14 GMT+09:00 Wolfram Sang <wsa@the-dreams.de>:
>
>> > Another try:
>> >
>> >> # core
>> >> obj-$(CONFIG_MMC_TMIO_CORE)           += tmio_mmc_core.o
>> >> # TMIO-builtin MMC
>> >> obj-$(CONFIG_MMC_TMIO)                += tmio_mmc_original.o
>> >> # Reneses
>> >> obj-$(CONFIG_MMC_SDHI)                += tmio_mmc_sdhi_core.o
>> >> # UniPhier
>> >> obj-$(CONFIG_MMC_UNIPHIER)            += tmio_mmc_uniphier.o
>> >
>> > I left the Kconfig symbols for easier updating and concentrated on the
>> > filenames for now.
>> >
>>
>> Looks good to me!
>
> Yamada-san?

Basically, looks good to me,
although I'd like to know our goal before we agree with it.

How about CONFIG names and functions?
Prefix CONFIG options with CONFIG_MMC_TMIO ?


# core
obj-$(CONFIG_MMC_TMIO_CORE)           += tmio_mmc_core.o
# TMIO-builtin MMC
obj-$(CONFIG_MMC_TMIO)                += tmio_mmc_original.o
# Reneses
obj-$(CONFIG_MMC_TMIO_SDHI)           += tmio_mmc_sdhi_core.o
# UniPhier
obj-$(CONFIG_MMC_TMIO_UNIPHIER)       += tmio_mmc_uniphier.o


Or, like follows?

# core
obj-$(CONFIG_MMC_TMIO)                += tmio_mmc.o
# TMIO-builtin MMC
obj-$(CONFIG_MMC_TMIO_ORIGINAL)       += tmio_mmc_original.o
# Reneses
obj-$(CONFIG_MMC_TMIO_SDHI)           += tmio_mmc_sdhi_core.o
# UniPhier
obj-$(CONFIG_MMC_TMIO_UNIPHIER)       += tmio_mmc_uniphier.o



What should I do for function names?
Shall I start with tmio_mmc_uniphier_ ?




-- 
Best Regards
Masahiro Yamada

Re: More TMIO MMC variant. What is a preferred name?

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 894 bytes --]

Yamada-san, Ulf,

> How about CONFIG names and functions?
> Prefix CONFIG options with CONFIG_MMC_TMIO ?

To be precise, I'd favour this:

# core
obj-$(CONFIG_MMC_TMIO_CORE)           += tmio_mmc_core.o
# TMIO-builtin MMC
obj-$(CONFIG_MMC_TMIO_ORIGINAL)       += tmio_mmc_original.o
# Reneses
obj-$(CONFIG_MMC_TMIO_SDHI_CORE)      += tmio_mmc_sdhi_core.o
# UniPhier
obj-$(CONFIG_MMC_TMIO_UNIPHIER)       += tmio_mmc_uniphier.o

But I am not sure if renaming CONFIG_MMC_TMIO to
CONFIG_MMC_TMIO_ORIGINAL is a regression risk?

> What should I do for function names?
> Shall I start with tmio_mmc_uniphier_ ?

I'll leave this to you and Ulf. If we e.g. rename "renesas_sdhi_core.c"
to "tmio_mmc_sdhi_core.c", I would still leave the function names as is
(prefixed with 'renesas_sdhi_*'). I wouldn't mind your functions
starting with "uniphier_mmc_" even, but that's just me.

Thanks,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]