On 03/01/2018 11:44 AM, Ulf Hansson wrote:

On 1 March 2018 at 10:57, Ludovic BARRE [off-list ref] wrote:

quoted

Hi Ulf

On 03/01/2018 10:06 AM, Ulf Hansson wrote:

quoted

Hi Ludovic,

On 28 February 2018 at 16:47, Ludovic Barre [off-list ref] wrote:

quoted

From: Ludovic Barre <redacted>

This patch serie adds support of stm32 SDMMC controller.
stm32h7 is the first SoC to use stm32 SDMMC controller
(previous SoC had pl180 controller).

I am a not convinced this isn't a new improved variant of the pl180.
According to register layout and the code you submitted in patch2,
there are great similarities to pl180 and the mmci driver.

In fact, ST designers which created stm32-sdmmc hardware block from scratch
are the same which have done the modifications on pl180 variant (stm32
legacy f4, f7).
So some registers or bits name seem identical (or strongly inspirited) but
the engine and features are different.

Well, in that case, I assume the driver would also need work
differently, but when looking at the code in patch2 this doesn't seem
to be the case.

I understand why you push to avoid drivers multiplication. But I'm
scared to squash 2 different hardware block which are their own roadmap
in the same drivers. I fear that it complicates the features management 
and maintenance of this driver.

there are some difference:
-the stm32-sdmmc use an internal dma (stm32-sdmmc is master on the bus), 
-idma alignment constraint.
-idma transfer mode single buffer, double buffer... (future evolution)
-need a command to stop transmission for data state machine
-same bit naming could have offset, mask width or set in different register.
=> I will try to synthesis register difference in a document
-voltage switch sequence
-delay block: calibration, tunning (sdr104)
-reset line required
-the same feature have not the same impact, example ddr mode
stm32:no bypass clk, activated in clk register
pl180: clkreg_neg_edge_enable, activated in datactrl register,
...

stm32-sdmmc is just at begining of its life cycle. Each revision of this 
block increases the difference with pl180. Today, I've just push the 
minimal driver to start a request, but I've not yet send all features of 
this revision like sdio, sdr104 support. After this revision there 
already are 2 other revisions in the pipe (at short term).

I am Out of Office with limited access to my e-mail till 2018 march 12th
I'll try to think about it on ski slope. euh, in fact no just ski and 
enjoy ;-)

BR
Ludo

quoted

You could find the datasheet of STM32H7x3 on:
http://www.st.com/content/ccc/resource/technical/document/reference_manual/group0/c9/a3/76/fa/55/46/45/fa/DM00314099/files/DM00314099.pdf/jcr:content/translations/en.DM00314099.pdf

Chapters: 55 Secure digital input/output MultiMediaCard interface
(SDMMC)

Thanks for sharing this. However this confirms my view, it looks
exactly as a new improved mmci variant. :-)

quoted

This hardware block has own roadmap and some features are already in the
pipe for next SoC.

That's fine. I don't have a problem extending the mmci driver, even
several times, as to cope with new revisions.

quoted

For code design: like I also worked on pl180 in the past :-)
my code is inspirited of this driver.

Right, that may explain things a bit.

However, besides a re-name of the registers, I really think that the
code execution, dealing with IRQs etc, is very similar to the mmci
driver. Isn't it?

So, I think it's at least worth to give it a go with the mmci driver
first, to see if we can get it to work.

I guess you understand why I am pushing!? This is about maintenance -
and I really want to avoid having a yet another driver to maintain,
unless we can extend an existing one.

[...]

Kind regards
Uffe