Thread (15 messages) 15 messages, 7 authors, 2016-02-15

Re: [PATCH v5] mmc: OCTEON: Add host driver for OCTEON MMC controller

From: Ulf Hansson <hidden>
Date: 2016-02-11 09:12:41
Also in: linux-mips, linux-mmc, lkml

On 11 February 2016 at 09:24, Matt Redfearn [off-list ref] wrote:
Hi,
I will split the DT binding document into a separate patch, and move the
legacy FDT patch-up code to
arch/mips/cavium-octeon/octeon-platform.c as suggested by Florian.

Ulf, your objections to the structure of the DT and driver were the only
driver for the changes v4->v5.
I changed the DT binding and the structure of the driver to more closely
resemble the atmel-mci driver, which has the same concept of one controller,
multiple slots.
I will review the driver code more closely in the next version, when
you have separated the DT binding doc and put the legacy part in
machine folder. Please keep me on cc on all patches.

Regarding "multiple slot" support. Don't follow other existing
examples in other mmc host drivers, as I find it highly unlikely that
the core will ever support this. *If* that changes, then the mmc host
driver may adopt this, but not until that.

Kind regards
Uffe
Thanks,
Matt


On 10/02/16 20:07, David Daney wrote:
quoted
On 02/10/2016 11:01 AM, Ulf Hansson wrote:
quoted
On 10 February 2016 at 18:36, Matt Redfearn [off-list ref]
wrote:
quoted
From: Aleksey Makarov <redacted>

The OCTEON MMC controller is currently found on cn61XX and cnf71XX
devices.  Device parameters are configured from device tree data.

eMMC, MMC and SD devices are supported.

Tested-by: Aaro Koskinen <aaro.koskinen@iki.fi>
Signed-off-by: Chandrakala Chavva <redacted>
Signed-off-by: David Daney <redacted>
Signed-off-by: Aleksey Makarov <redacted>
Signed-off-by: Leonid Rosenboim <redacted>
Signed-off-by: Peter Swain <redacted>
Signed-off-by: Aaron Williams <redacted>
Signed-off-by: Matt Redfearn <redacted>
---
v5:
Incoroprate comments from review
http://patchwork.linux-mips.org/patch/9558/
- Use standard <bus-width> property instead of <cavium,bus-max-width>.
- Use standard <max-frequency> property instead of <spi-max-frequency>.
- Add octeon_mmc_of_parse_legacy function to deal with the above
   properties, since many devices have shipped with those properties
   embedded in firmware.
- Allow the <vmmc-supply> binding in addition to the legacy
   <gpios-power>.
- Remove the secondary driver for each slot.
- Use core gpio cd/wp handling

Seems like you decided to ignore most comments realted to the DT
bindings from the earlier version.
Although, let's discuss this one more time.

I think you may have misread the patch.  The DT bindings have been
changed based on the feedback we received on v4.
quoted
Therefore I recomend you to split this patch. DT documentation should
be a separate patch preceeding the actual mmc driver patch.

You may have missed it the first time it was posted, but the legacy DT
bindings have been around for a while.

See:

https://lists.ozlabs.org/pipermail/devicetree-discuss/2012-May/015482.html

quoted
The DT patch needs to be acked by the DT maintainers.

The legacy DT has been deployed in firmware for several years now.  We
are adding more "modern" bindings, and the DT maintainers are
encouraged to review that portion, but the legacy is what it is and it
isn't changing.
quoted
Until we somewhat agreed on the DT parts, I am going to defer the
in-depth review of the driver code as I have limited bandwidth.
As I stated above, the legacy DT bindings are not changing and must be
supported.  Waiting for legacy DT bindings to change is equivalent to
infinite deferral.
quoted
Does that make sense to you?
I understand why you would say this.  However, I think it doesn't
fully take into account the need to support devices that have already
been deployed.

That said, Matt really needs to get the DT maintainers reviewing the new
DT bindings.


David Daney
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help